Re: [Qemu-devel] [PATCH for-1.4 04/12] kvm: Create kvm_arch_vcpu_id() function

[Eric Blake]

On 01/18/2013 09:40 AM, Eduardo Habkost wrote:
> On Fri, Jan 18, 2013 at 09:11:42AM -0700, Eric Blake wrote:
>> On 01/18/2013 07:20 AM, Eduardo Habkost wrote:
>>>> Could you suggest a text for me to add please?
>>>
>>> "The argument passed to KVM_CREATE_VCPU now has 'unsigned long' type
>>> instead of 'int', as expected by the Linux ioctl() syscall. Maybe an int
>>> works on most or all architectures supporting KVM, but it is safer to
>>> use an appropriate 'unsigned long' parameter."
>>
>> Interestingly enough, while the Linux syscall uses 'unsigned long', the
>> POSIX definition of ioctl() uses 'int'; so the Linux kernel is already
>> constrained to never use an ioctl value that doesn't fit within 'int',
> 
> Really? What about the ioctl()s that get a pointer as argument on
> architectures where pointers don't fit in an int?
> 
> Do you have a pointer to the POSIX definition you are talking about?
> 
> Note that I'm talking about the the extra ioctl() argument, not the
> ioctl() number (that is an unsigned int in the kernel code).

Okay, now you made me go back and check sources.

POSIX 2008 says:
#include <stropts.h>
int ioctl(int fildes, int request, ... /* arg */);

Gnulib says this about a bug that it works around:
@item
On glibc platforms, the second parameter is of type @code{unsigned long}
rather than @code{int}.

But gnulib also suggests using <sys/ioctl.h> instead of the POSIX header
<stropts.h> for getting ioctl(), because <stropts.h> was declared
obsolete in POSIX 2008 and was never implemented in glibc.

Sure enough, looking at Fedora 18 /usr/include/sys/ioctl.h, I still see:
extern int ioctl (int __fd, unsigned long int __request, ...) __THROW;

Meanwhile, you are correct that the kernel defines request as 32 bits:
linux.git:include/uapi/asm-generic/ioctl.h
/* ioctl command encoding: 32 bits total, command in lower 16 bits,
 * size of the parameter structure in the lower 14 bits of the
 * upper 16 bits.
 * Encoding the size of the parameter structure in the ioctl request
 * is useful for catching programs compiled with old versions
 * and to avoid overwriting user space outside the user buffer area.
 * The highest 2 bits are reserved for indicating the ``access mode''.
 * NOTE: This limits the max parameter size to 16kB -1 !
 */

> 
>> and glibc is already responsible for ensuring that argument promotion of
>> an int doesn't change the behavior of ioctl() in libc when converting it
>> over to the unsigned long syscall semantics expected by the kernel.

So a more precise wording of this is:

glibc is already responsible from converting the 'unsigned long int' of
the user declaration back into the 'unsigned int' that the kernel
expects for the second argument.  The third argument (when present), is
generally treated as a pointer (of size appropriate for the
architecture).  Although there _might_ be an ioctl that uses it directly
as an integer instead of dereferencing it as a pointer, those would be
the exceptions to the rule.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature