Hi Dean,
> Add global mutex lock to prevent reentrancy in the hci_uart_tty_ioctl()
> ioctl handling. This prevents concurrency of all function calls within
> hci_uart_tty_ioctl handling including hci_uart_set_proto().
>
> Also removed multiple return statements in hci_uart_tty_ioctl call and
> added a single return statement since the mutex needs to be unlocked
> before exiting the function.
can we split this into two please. First change the error handling path and then introduce the mutex.
>
> Signed-off-by: Vignesh Raman <Vignesh_Raman@xxxxxxxxxx>
> Signed-off-by: Dean Jenkins <Dean_Jenkins@xxxxxxxxxx>
> Signed-off-by: Rajeev Kumar <rajeev_kumar@xxxxxxxxxx>
> ---
> drivers/bluetooth/hci_ldisc.c | 43 ++++++++++++++++++++++++++-----------------
> 1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 1e338d5..cf1883d 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -51,6 +51,8 @@
>
> #define VERSION "2.3"
>
> +static DEFINE_MUTEX(ioctl_mutex);
> +
> static const struct hci_uart_proto *hup[HCI_UART_MAX_PROTO];
>
> int hci_uart_register_proto(const struct hci_uart_proto *p)
> @@ -685,7 +687,7 @@ static int hci_uart_tty_ioctl(struct tty_struct *tty, struct file *file,
> unsigned int cmd, unsigned long arg)
> {
> struct hci_uart *hu = tty->disc_data;
> - int err = 0;
> + int result = 0;
I am fine keeping this err. I think renaming it to result does not help.
>
> BT_DBG("");
>
> @@ -693,45 +695,52 @@ static int hci_uart_tty_ioctl(struct tty_struct *tty, struct file *file,
> if (!hu)
> return -EBADF;
>
> + mutex_lock(&ioctl_mutex);
> switch (cmd) {
> case HCIUARTSETPROTO:
> if (!test_bit(HCI_UART_PROTO_SET, &hu->flags)) {
> - err = hci_uart_set_proto(hu, arg);
> - if (!err)
> + result = hci_uart_set_proto(hu, arg);
> + if (!result)
> set_bit(HCI_UART_PROTO_SET, &hu->flags);
> - else
> - return err;
> } else
> - return -EBUSY;
> + result = -EBUSY;
> break;
>
> case HCIUARTGETPROTO:
> if (test_bit(HCI_UART_PROTO_SET, &hu->flags))
> - return hu->proto->id;
> - return -EUNATCH;
> + result = hu->proto->id;
> + else
> + result = -EUNATCH;
> + break;
>
> case HCIUARTGETDEVICE:
> if (test_bit(HCI_UART_REGISTERED, &hu->flags))
> - return hu->hdev->id;
> - return -EUNATCH;
> + result = hu->hdev->id;
> + else
> + result = -EUNATCH;
> + break;
>
> case HCIUARTSETFLAGS:
> if (test_bit(HCI_UART_PROTO_SET, &hu->flags))
> - return -EBUSY;
> - err = hci_uart_set_flags(hu, arg);
> - if (err)
> - return err;
> + result = -EBUSY;
> + else {
> + result = hci_uart_set_flags(hu, arg);
> + if (result)
> + return result;
> + }
> break;
>
> case HCIUARTGETFLAGS:
> - return hu->hdev_flags;
> + result = hu->hdev_flags;
> + break;
>
> default:
> - err = n_tty_ioctl_helper(tty, file, cmd, arg);
> + result = n_tty_ioctl_helper(tty, file, cmd, arg);
> break;
> }
> + mutex_unlock(&ioctl_mutex);
>
> - return err;
> + return result;
> }
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
