[PATCH] tty: vt: selection: fix soft lockup in paste_selection()

[PATCH] tty: vt: selection: fix soft lockup in paste_selection()

[Yi Yang]

Soft lockup occurs when vt device used n_null ldisc, n_null_receivebuf()
is not implemented in null_ldisc. So tty_ldisc_receive_buf always return
0 in paste_selection(), this cause deadloop and cause soft lockup.

This can be reproduced as follows:
  int ldisc = 0x1b; // 0x1b is n_null
  struct{
  	char subcode;
  	struct tiocl_selection sel;
  } data;
  date.subcode = TIOCL_SETSEL;
  data.sel.xs = 0;
  data.sel.xe = 1;
  data.sel.ys = 0;
  data.sel.ye = 1;
  data.sel.sel_mode = TIOCL_SELCHAR;
  char bytes[2] = {TIOCL_PASTESEL, 0};
  open("ttyxx", O_RDWR) // open a vt device
  ioctl(fd, TIOCSETD, &ldisc) // set ldisc to n_null
  ioctl(fd, TIOCLINUX, &data.subcode);
  ioctl(fd, TIOCLINUX, bytes); // cause deadloop

Fix soft lockup by check if ldisc in paste_selection() is n_null.

Link: https://lore.kernel.org/all/000000000000fe769905d315a1b7@google.com/
Fixes: 8a8dabf2dd68 ("tty: handle the case where we cannot restore a line discipline")
Signed-off-by: Yi Yang <yiyang13@huawei.com>
---
 drivers/tty/vt/selection.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 6ef22f01cc51..9ba7f66fcf05 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -388,6 +388,12 @@ int paste_selection(struct tty_struct *tty)
 	ld = tty_ldisc_ref_wait(tty);
 	if (!ld)
 		return -EIO;	/* ldisc was hung up */
+
+	/* tty_ldisc_receive_buf() will not do anything when ldisc is n_null*/
+	if (ld->ops->num == N_NULL) {
+		tty_ldisc_deref(ld);
+		return -EIO;
+	}
 	tty_buffer_lock_exclusive(&vc->port);
 
 	add_wait_queue(&vc->paste_wait, &wait);
-- 
2.17.1

Re: [PATCH] tty: vt: selection: fix soft lockup in paste_selection()

[Jiri Slaby]

On 14. 08. 23, 6:01, Yi Yang wrote:
> Soft lockup occurs when vt device used n_null ldisc, n_null_receivebuf()
> is not implemented in null_ldisc. So tty_ldisc_receive_buf always return
> 0 in paste_selection(), this cause deadloop and cause soft lockup.
> 
> This can be reproduced as follows:
>    int ldisc = 0x1b; // 0x1b is n_null
>    struct{
>    	char subcode;
>    	struct tiocl_selection sel;
>    } data;
>    date.subcode = TIOCL_SETSEL;
>    data.sel.xs = 0;
>    data.sel.xe = 1;
>    data.sel.ys = 0;
>    data.sel.ye = 1;
>    data.sel.sel_mode = TIOCL_SELCHAR;
>    char bytes[2] = {TIOCL_PASTESEL, 0};
>    open("ttyxx", O_RDWR) // open a vt device
>    ioctl(fd, TIOCSETD, &ldisc) // set ldisc to n_null
>    ioctl(fd, TIOCLINUX, &data.subcode);
>    ioctl(fd, TIOCLINUX, bytes); // cause deadloop
> 
> Fix soft lockup by check if ldisc in paste_selection() is n_null.

Ugh, no. What if another ldisc returns with 0 too?

So instead, what about checking for progress instead of checking a 
particular ldisc?

> Link: https://lore.kernel.org/all/000000000000fe769905d315a1b7@google.com/
> Fixes: 8a8dabf2dd68 ("tty: handle the case where we cannot restore a line discipline")
> Signed-off-by: Yi Yang <yiyang13@huawei.com>
> ---
>   drivers/tty/vt/selection.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
> index 6ef22f01cc51..9ba7f66fcf05 100644
> --- a/drivers/tty/vt/selection.c
> +++ b/drivers/tty/vt/selection.c
> @@ -388,6 +388,12 @@ int paste_selection(struct tty_struct *tty)
>   	ld = tty_ldisc_ref_wait(tty);
>   	if (!ld)
>   		return -EIO;	/* ldisc was hung up */
> +
> +	/* tty_ldisc_receive_buf() will not do anything when ldisc is n_null*/
> +	if (ld->ops->num == N_NULL) {
> +		tty_ldisc_deref(ld);
> +		return -EIO;
> +	}
>   	tty_buffer_lock_exclusive(&vc->port);
>   
>   	add_wait_queue(&vc->paste_wait, &wait);

-- 
js
suse labs

Re: [PATCH] tty: vt: selection: fix soft lockup in paste_selection()

[yiyang (D)]

On 2023/8/14 14:32, Jiri Slaby wrote:
> On 14. 08. 23, 6:01, Yi Yang wrote:
>> Soft lockup occurs when vt device used n_null ldisc, n_null_receivebuf()
>> is not implemented in null_ldisc. So tty_ldisc_receive_buf always return
>> 0 in paste_selection(), this cause deadloop and cause soft lockup.
>>
>> This can be reproduced as follows:
>>    int ldisc = 0x1b; // 0x1b is n_null
>>    struct{
>>        char subcode;
>>        struct tiocl_selection sel;
>>    } data;
>>    date.subcode = TIOCL_SETSEL;
>>    data.sel.xs = 0;
>>    data.sel.xe = 1;
>>    data.sel.ys = 0;
>>    data.sel.ye = 1;
>>    data.sel.sel_mode = TIOCL_SELCHAR;
>>    char bytes[2] = {TIOCL_PASTESEL, 0};
>>    open("ttyxx", O_RDWR) // open a vt device
>>    ioctl(fd, TIOCSETD, &ldisc) // set ldisc to n_null
>>    ioctl(fd, TIOCLINUX, &data.subcode);
>>    ioctl(fd, TIOCLINUX, bytes); // cause deadloop
>>
>> Fix soft lockup by check if ldisc in paste_selection() is n_null.
> 
> Ugh, no. What if another ldisc returns with 0 too?
> 
> So instead, what about checking for progress instead of checking a 
> particular ldisc?
> 

Sorry, my previous analysis have problem. The real reason why
tty_ldisc_receive_buf() returns 0 is that n_null ldisc does not
assign a value to receive_room. Therefore, min_t(int, count,
ld->tty->receive_room) is always 0 in the tty_ldisc_receive_buf().
Maybe i should check the receive_room size instead of checking the
specific ldisc, check if receive_room size is 0 can fix this issue
passed my test.

>> Link: 
>> https://lore.kernel.org/all/000000000000fe769905d315a1b7@google.com/
>> Fixes: 8a8dabf2dd68 ("tty: handle the case where we cannot restore a 
>> line discipline")
>> Signed-off-by: Yi Yang <yiyang13@huawei.com>
>> ---
>>   drivers/tty/vt/selection.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
>> index 6ef22f01cc51..9ba7f66fcf05 100644
>> --- a/drivers/tty/vt/selection.c
>> +++ b/drivers/tty/vt/selection.c
>> @@ -388,6 +388,12 @@ int paste_selection(struct tty_struct *tty)
>>       ld = tty_ldisc_ref_wait(tty);
>>       if (!ld)
>>           return -EIO;    /* ldisc was hung up */
>> +
>> +    /* tty_ldisc_receive_buf() will not do anything when ldisc is 
>> n_null*/
>> +    if (ld->ops->num == N_NULL) {
>> +        tty_ldisc_deref(ld);
>> +        return -EIO;
>> +    }
>>       tty_buffer_lock_exclusive(&vc->port);
>>       add_wait_queue(&vc->paste_wait, &wait);
> 

-- 
Yi Yang

Re: [PATCH] tty: vt: selection: fix soft lockup in paste_selection()

[yiyang (D)]

On 2023/8/14 16:52, Yi Yang wrote:
> On 2023/8/14 14:32, Jiri Slaby wrote:
>> On 14. 08. 23, 6:01, Yi Yang wrote:
>>> Soft lockup occurs when vt device used n_null ldisc, n_null_receivebuf()
>>> is not implemented in null_ldisc. So tty_ldisc_receive_buf always return
>>> 0 in paste_selection(), this cause deadloop and cause soft lockup.
>>>
>>> This can be reproduced as follows:
>>>    int ldisc = 0x1b; // 0x1b is n_null
>>>    struct{
>>>        char subcode;
>>>        struct tiocl_selection sel;
>>>    } data;
>>>    date.subcode = TIOCL_SETSEL;
>>>    data.sel.xs = 0;
>>>    data.sel.xe = 1;
>>>    data.sel.ys = 0;
>>>    data.sel.ye = 1;
>>>    data.sel.sel_mode = TIOCL_SELCHAR;
>>>    char bytes[2] = {TIOCL_PASTESEL, 0};
>>>    open("ttyxx", O_RDWR) // open a vt device
>>>    ioctl(fd, TIOCSETD, &ldisc) // set ldisc to n_null
>>>    ioctl(fd, TIOCLINUX, &data.subcode);
>>>    ioctl(fd, TIOCLINUX, bytes); // cause deadloop
>>>
>>> Fix soft lockup by check if ldisc in paste_selection() is n_null.
>> 
>> Ugh, no. What if another ldisc returns with 0 too?
>> 
>> So instead, what about checking for progress instead of checking a 
>> particular ldisc?
>> 
> 
> Sorry, my previous analysis have problem. The real reason why
> tty_ldisc_receive_buf() returns 0 is that n_null ldisc does not
> assign a value to receive_room. Therefore, min_t(int, count,
> ld->tty->receive_room) is always 0 in the tty_ldisc_receive_buf().
> Maybe i should check the receive_room size instead of checking the
> specific ldisc, check if receive_room size is 0 can fix this issue
> passed my test.
> 

Check receive_room size no a good idear, receive_room may also be 0 when
ldisc is n_tty. Do you have any idear if do not check the particular ldisc?

>>> Link: 
>>> https://lore.kernel.org/all/000000000000fe769905d315a1b7@google.com/
>>> Fixes: 8a8dabf2dd68 ("tty: handle the case where we cannot restore a 
>>> line discipline")
>>> Signed-off-by: Yi Yang <yiyang13@huawei.com>
>>> ---
>>>   drivers/tty/vt/selection.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
>>> index 6ef22f01cc51..9ba7f66fcf05 100644
>>> --- a/drivers/tty/vt/selection.c
>>> +++ b/drivers/tty/vt/selection.c
>>> @@ -388,6 +388,12 @@ int paste_selection(struct tty_struct *tty)
>>>       ld = tty_ldisc_ref_wait(tty);
>>>       if (!ld)
>>>           return -EIO;    /* ldisc was hung up */
>>> +
>>> +    /* tty_ldisc_receive_buf() will not do anything when ldisc is 
>>> n_null*/
>>> +    if (ld->ops->num == N_NULL) {
>>> +        tty_ldisc_deref(ld);
>>> +        return -EIO;
>>> +    }
>>>       tty_buffer_lock_exclusive(&vc->port);
>>>       add_wait_queue(&vc->paste_wait, &wait);
>> 
-- 
Yi Yang