linux-kernel - Re: [PATCH 03/12] serial_core: Handle TIOC[GS]RS485 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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPybu_1vXc+=dVbradArvFYiFBy2c=0tTM7RAHDhycCkQQv_5g@mail.gmail.com>
Date:	Wed, 8 Oct 2014 22:43:38 +0200
From:	Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
To:	Alan Cox <alan@...ux.intel.com>
Cc:	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	linux-serial@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.cz>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 03/12] serial_core: Handle TIOC[GS]RS485 ioctls.

Hello Alan

This patchset adds no extra locking features, if the drivers did not
implement a locking mechanism (and none did) there is chance of
conflict.

I can add a call to lock/unlock around uart_[gs]et_rs485_config. And
then, inside the drivers, use the lock when the structure is used.

I would prefer to add it as new patches in the patchset, so in case
this adds some bugs they can be bisected easily.

Thanks for your fast reply



On Wed, Oct 8, 2014 at 10:11 PM, Alan Cox <alan@...ux.intel.com> wrote:
> On Wed, 2014-10-08 at 21:57 +0200, Ricardo Ribalda Delgado wrote:
>> The following drivers: 8250_core, atmel_serial, max310x, mcf, omap-serial
>> and sci16is7xx implement code to handle RS485 ioctls.
>
>>
>> +static int uart_get_rs485_config(struct uart_port *port,
>> +                      struct serial_rs485 __user *rs485)
>> +{
>> +     if (!port->rs485_config)
>> +             return -ENOIOCTLCMD;
>> +
>> +     if (copy_to_user(rs485, &port->rs485, sizeof(port->rs485)))
>> +             return -EFAULT;
>> +     return 0;
>> +}
>> +
>> +static int uart_set_rs485_config(struct uart_port *port,
>> +                      struct serial_rs485 __user *rs485_user)
>> +{
>> +     struct serial_rs485 rs485;
>> +     int ret;
>> +
>> +     if (!port->rs485_config)
>> +             return -ENOIOCTLCMD;
>> +
>> +     if (copy_from_user(&rs485, rs485_user, sizeof(rs485_user)))
>> +             return -EFAULT;
>> +
>> +     ret = port->rs485_config(port, &rs485);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (copy_to_user(rs485_user, &port->rs485, sizeof(port->rs485)))
>> +             return -EFAULT;
>> +
>> +     return 0;
>> +
>
> What is the locking between setting/getting/driver use of the config ?
> This really needs a lock (termios sem I think is perhaps appropriate
> given when the values are normally referenced).
>
> Alan
>
>



-- 
Ricardo Ribalda
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/