linux-kernel - Re: [PATCH v3] USB: serial: xr: Add TIOCGRS485 and TIOCSRS485 ioctls

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: <f1e5eb1c-ed1f-96d7-ecf5-69b467ed8339@iki.fi>
Date:   Tue, 14 Mar 2023 10:00:22 +0200
From:   Jarkko Sonninen <kasper@....fi>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Johan Hovold <johan@...nel.org>, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] USB: serial: xr: Add TIOCGRS485 and TIOCSRS485 ioctls

On 3/14/23 09:37, Greg Kroah-Hartman wrote:
> On Tue, Mar 14, 2023 at 09:00:01AM +0200, Jarkko Sonninen wrote:
>> Add support for RS-485 in Exar USB adapters.
>> RS-485 mode is controlled by TIOCGRS485 and TIOCSRS485 ioctls.
>> Gpio mode register is set to enable RS-485.
>>
>> Signed-off-by: Jarkko Sonninen <kasper@....fi>
>> ---
>>
>> In this version only rs485.flags are stored to state.
>> There is no locking as only one bit of the flags is used.
>> ioctl returns -ENOIOCTLCMD as the actual error handling is in tty code.
> And the difference between previous versions?  Take a look at the
> documentation for how to better describe version differences please.

I'll try to be more precise in the future.

>> +	memset(&rs485, 0, sizeof(rs485));
> But you just copied this from userspace, why zero it out again?  Is that
> to be expected (I really don't remember, sorry).


serial.h says

/*
  * Serial interface for controlling RS485 settings on chips with suitable
  * support. Set with TIOCSRS485 and get with TIOCGRS485 if supported by 
your
  * platform. The set function returns the new state, with any 
unsupported bits
  * reverted appropriately.
  */

This may mean only the flags, but I decided to zero the other 
unsupported fields as well.


> Anyway, just minor comments, I'll let others review it as well.
>
> thanks,
>
> greg k-h

     - Jarkko