evdev: modify the return value of ioctl different from linux

[ankohuu_outlook.com]

Summary

some evdev ioctl commands return different from that of Linux,
mainly variable-length ioctls.

[wulf]

sys/dev/evdev/cdev.c

580–583

Could you use limit variable to be consistent with existing code?

Also couple words about why strlen/memcpy is used instead of strlcopy would be nice.

624

Do EVIOCGMTSLOTS returns the length?

667

IMO returning of length in len or limit field rather in td->td_retval would make the code cleaner.

[ankohuu_outlook.com]

sys/dev/evdev/cdev.c

580–583

Could you use limit variable to be consistent with existing code?

ok

Also couple words about why strlen/memcpy is used instead of strlcopy would be nice.

I missed the reason for this modification, on Linux when ioctl is EVIOCGNAME | EVIOCGPHYS | EVIOCGUNIQ, it use str_to_user to copy data.
str_to_user has a different behavior as strlcpy in FreeBSD kernel, when args->len is equal or smaller than the length of source string,
str_to_user copy "len" bytes to dst and strlcpy copy "len - 1" to dst and the last one is zero

static int str_to_user(const char *str, unsigned int maxlen, void __user *p)         
{                                                                                
    int len;                                                                     
                                                                                 
    if (!str)                                                                    
        return -ENOENT;                                                          
                                                                                 
    len = strlen(str) + 1;                                                       
    if (len > maxlen)                                                            
        len = maxlen;                                                            
                                                                                 
    return copy_to_user(p, str, len) ? -EFAULT : len;                            
}

624

Do EVIOCGMTSLOTS returns the length?

It returns 0 when correct,
when device is not MT or ev_code passed is wrong it returns EINVAL,
when something wrong happened in copy, it returns EFAULT.

667

IMO returning of length in len or limit field rather in td->td_retval would make the code cleaner.

I didn't understand, do you mean I need change return value's meaning of evdev_ioctl_eviocgbit, now it represents operation EVIOCGBIT's error code,
should I mix length meaning in it for return actual length to funcion evdev_do_ioctl?
Or should I change evdev_ioctl_eviocgbit's argument len to &len, in order to return actual length to evdev_do_ioctl?
I think this is a syscall and when no error accurs I need a positive return, I have to use td->td_retval[0] to store it.

[ankohuu_outlook.com]

1. use limit variable to be consistent with existing code
2. explain why I modify strlcpy with memcpy in order to acts just as Linux's str_to_user do.

on Linux when ioctl is EVIOCGNAME | EVIOCGPHYS | EVIOCGUNIQ, it use str_to_user to copy data.
str_to_user has a different behavior as strlcpy in FreeBSD kernel, when args->len is equal or smaller than the length of source string,
str_to_user copy "len" bytes to dst and strlcpy copy "len - 1" to dst and the last one is zero

[wulf]

I ll handle my comments at my own as you are not a committer.

One question still left: Do you know one piece of software that depend on length returned by ioctl()?

sys/dev/evdev/cdev.c

580–583

This requires one line comment as it is not obvious and refers Linux sources.

624

This requires one line comment to as it does not matches other cases.

667

Or should I change evdev_ioctl_eviocgbit's argument len to &len

I meant exactly that. Passing &len as output parameter looks cleaner than passing td as output parameter. And does not require 1 extra argument to evdev_ioctl_eviocgbit().

[ankohuu_outlook.com]

In D28218#631428, @wulf wrote:

I ll handle my comments at my own as you are not a committer.

Thank you, I got you point in return value, I think we call evdev_ioctl_eviocgbit using return, this callprocess will finish in evdev_ioctl_eviocgbit, so I don't know whether return the length to upper layer is necessary

One question still left: Do you know one piece of software that depend on length returned by ioctl()?

I know one and that's why I do this thing.We run Imprivata Linux binary in FreeBSD, when login their python code read some hardware such as fingerprint, smart card or other for authentication,
some device failed because the return value，this is commercial software so we don't have code, I do some ktrace and reverse engineering to find and fix this bug.