lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <77db0332-4fd1-fb5f-8c22-10653139f3e7@metafoo.de>
Date:   Sat, 27 Mar 2021 13:00:18 +0100
From:   Lars-Peter Clausen <lars@...afoo.de>
To:     Alexandru Ardelean <ardeleanalex@...il.com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc:     Jonathan Cameron <jic23@...23.retrosnub.co.uk>,
        "Sa, Nuno" <Nuno.Sa@...log.com>,
        "zzzzArdelean, zzzzAlexandru" <alexandru.Ardelean@...log.com>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-iio <linux-iio@...r.kernel.org>,
        "Hennerich, Michael" <Michael.Hennerich@...log.com>,
        "Bogdan, Dragos" <Dragos.Bogdan@...log.com>
Subject: Re: [PATCH v6 20/24] iio: buffer: add ioctl() to support opening
 extra buffers for IIO device

On 3/24/21 10:10 AM, Alexandru Ardelean wrote:
> On Tue, Mar 23, 2021 at 1:35 PM Jonathan Cameron
> <Jonathan.Cameron@...wei.com> wrote:
>>
[..]
>>>
>>> Continuing a bit with the original IIO buffer ioctl(), I talked to
>>> Lars a bit over IRC.
>>> And there was an idea/suggestion to maybe use a struct to pass more
>>> information to the buffer FD.
>>>
>>> So, right now the ioctl() just returns an FD.
>>> Would it be worth to extend this to a struct?
>>> What I'm worried about is that it opens the discussion to add more
>>> stuff to that struct.
>>>
>>> so now, it would be:
>>>
>>> struct iio_buffer_ioctl_data {
>>>              __u32 fd;
>>>              __u32 flags;   // flags for the new FD, which maybe we
>>> could also pass via fcntl()
>>> }
>>>
>>> anything else that we would need?
>>
>> I have a vague recollection that is is almost always worth adding
>> some padding to such ioctl data coming out of the kernel.  Gives
>> flexibility to safely add more stuff later without userspace
>> failing to allocate enough space etc.
>>
>> I'm curious though, because this feels backwards. I'd expect the
>> flags to be more useful passed into the ioctl? i.e. request
>> a non blocking FD?  Might want to mirror that back again of course.
> 

The struct can be used for both. Passing flags and buffer number in and fd out.

> Personally, I don't know.
> I don't have any experiences on this.
> 
> So, then I'll do a change to this ioctl() to use a struct.
> We can probably add some reserved space?
> 
> struct iio_buffer_ioctl_data {
>              __u32 fd;
>              __u32 flags;
>              __u32 reserved1;
>              __u32 reserved2;
> }

What to make sure of when using reserved fields is to check that they are 0. 
And reject the ioctl if they are not. This is the only way to ensure that old 
applications will continue to work if the struct is updated.

> 
> Lars was giving me some articles about ioctls.
> One idea was to maybe consider making them multiples of 64 bits.
> 
> But reading through one of the docs here:
>       https://www.kernel.org/doc/html/latest/driver-api/ioctl.html#interface-versions
> it discourages to do interface versions.
> 
> But I guess if we plan ahead with some reserved space, it might be
> somewhat fine.
> 
> I'm still a little green on this stuff.
> 
>>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ