Project

General

Profile

Actions

Bug #18255

open

ioctl zeroes the last buffer byte

Added by vihai (Daniele Orlandi) over 2 years ago. Updated over 2 years ago.

Status:
Open
Assignee:
-
Target version:
-
[ruby-core:105665]

Description

Hello,

I'm running ruby 2.7.4p191 on an armv7 linux and experimenting with GPIO_GET_LINEHANDLE_IOCTL ioctl.

The ioctl sanity check is triggered as if the buffer was too small however the size of the buffer passed to ioctl is correct.

io.rb:116:in `ioctl': return value overflowed string (ArgumentError)

If I append at least one byte to the buffer the ioctl does not raise an exception.

It seems that the last byte of the buffer is zeroed:

puts "SIZE=#{req.bytesize}"
req = req + "XXXXXXXXXX".b     
puts req.unpack("H*")       
fd.ioctl(GPIO_GET_LINEHANDLE_IOCTL, req)                        
puts req.unpack("H*")     
SIZE=364
[...]0000000000000058585858585858585858
[...]0000000600000058585858585858585800

I checked with a C program and the ioctl does not actually touch the buffer beyond the expected 364 bytes.
The ioctl number does encode 364 as size:

#include <stdio.h>
#include <linux/gpio.h>

void main()
{
  printf("SIZE=%d", _IOC_SIZE(GPIO_GET_LINEHANDLE_IOCTL));
}
SIZE=364

Updated by nobu (Nobuyoshi Nakada) over 2 years ago

  • Status changed from Open to Feedback

vihai (Daniele Orlandi) wrote:

The ioctl sanity check is triggered as if the buffer was too small however the size of the buffer passed to ioctl is correct.

io.rb:116:in `ioctl': return value overflowed string (ArgumentError)

If I append at least one byte to the buffer the ioctl does not raise an exception.

Do you mean that fd.ioctl(GPIO_GET_LINEHANDLE_IOCTL, "X"*364) raises the exception?

It seems that the last byte of the buffer is zeroed:

Yes, it's the sentinel byte added internally.

Updated by vihai (Daniele Orlandi) over 2 years ago

nobu (Nobuyoshi Nakada) wrote in #note-1:

Do you mean that fd.ioctl(GPIO_GET_LINEHANDLE_IOCTL, "X"*364) raises the exception?

"X"*364 is a request that makes ioctl fail and the SystemCallError is raised before the sanity check of the buffer.

However, given req as a proper request as a binary string:

> fd.ioctl(GPIO_GET_LINEHANDLE_IOCTL, req)
`ioctl': return value overflowed string (ArgumentError)

> fd.ioctl(GPIO_GET_LINEHANDLE_IOCTL, req + ' '.b)
Success

It seems that the last byte of the buffer is zeroed:

Yes, it's the sentinel byte added internally.

As I see from the source the sentinel byte is a 17 (decimal) and as far as I understand it should be outside the visible buffer.

Something else zeroes the last byte of the buffer (which happens to be the sentinel byte when the buffer is properly sized).

Updated by nobu (Nobuyoshi Nakada) over 2 years ago

  • Status changed from Feedback to Open
  • Backport changed from 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN to 2.6: REQUIRED, 2.7: REQUIRED, 3.0: REQUIRED

Found the bug.
Does this patch fix it?

diff --git a/io.c b/io.c
index 50c9fea62c9..052155205d6 100644
--- a/io.c
+++ b/io.c
@@ -10143,8 +10143,8 @@ setup_narg(ioctl_req_t cmd, VALUE *argp, int io_p)
 	    /* expand for data + sentinel. */
 	    if (slen < len+1) {
 		rb_str_resize(arg, len+1);
-		MEMZERO(RSTRING_PTR(arg)+slen, char, len-slen);
-		slen = len+1;
+		RSTRING_GETMEM(arg, ptr, slen);
+		MEMZERO(ptr+slen, char, len-slen);
 	    }
 	    /* a little sanity check here */
 	    ptr = RSTRING_PTR(arg);

Updated by vihai (Daniele Orlandi) over 2 years ago

nobu (Nobuyoshi Nakada) wrote in #note-3:

Found the bug.
Does this patch fix it?

Standby, I'm still setting up the environment to build for the embedded machine.

Updated by nobu (Nobuyoshi Nakada) over 2 years ago

ptr was reread just after the block, this is not a bug.

Updated by vihai (Daniele Orlandi) over 2 years ago

nobu (Nobuyoshi Nakada) wrote in #note-3:

Found the bug.
Does this patch fix it?

Here I am.

Apparently it doesn't.
I dug a bit deeper and I found that there are two issues that concur to this behavior:

  • ioctl_narg_len isn't properly extracting the buffer size from the ioctl number and defaults to DEFULT_IOCTL_NARG_LEN
  • When the original buffer is bigger than DEFULT_IOCTL_NARG_LEN it is not expanded to make room to the sentinel byte which is then overwrote with the string terminator.

The first issue is caused by <sys/ioctl.h> not defining _IOC_SIZE, ruby falls back to DEFULT_IOCTL_NARG_LEN. I guess you have to detect and include <linux/ioctl.h> or <asm/ioctl.h>.

The second may be patched like this:

--- io.c
+++ io.c.patched
@@ -9823,7 +9823,11 @@
                rb_str_resize(arg, len+1);
                MEMZERO(RSTRING_PTR(arg)+slen, char, len-slen);
                slen = len+1;
+           } else {
+               rb_str_resize(arg, slen+1);
+               slen++;
            }
+
            /* a little sanity check here */
            ptr = RSTRING_PTR(arg);
            ptr[slen - 1] = 17;

Lastly I guess that DEFULT is spelled incorrectly :)

Updated by nobu (Nobuyoshi Nakada) over 2 years ago

vihai (Daniele Orlandi) wrote in #note-6:

The first issue is caused by <sys/ioctl.h> not defining _IOC_SIZE, ruby falls back to DEFULT_IOCTL_NARG_LEN. I guess you have to detect and include <linux/ioctl.h> or <asm/ioctl.h>.

That means, linux_iocparm_len is not defined?
Whether _IOC_SIZE is defined seems depending on versions/architectures.
At least, the following code can compile and prints the expected values on Ubuntu 21.10 x86_64.

#include <sys/ioctl.h>
#include <linux/gpio.h>
#include <stdio.h>
int main(void)
{
    const size_t n = GPIO_GET_LINEHANDLE_IOCTL;
    printf("%#zx => %#zx\n", n, _IOC_SIZE(n));
    // 0xc16cb403 => 0x16c
    return 0;
}

The second may be patched like this:

As the buffer is supposed to be overwritten, it is doubtful to be considered a bug.

Lastly I guess that DEFULT is spelled incorrectly :)

Yes, definitely ;)

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0