linux-kernel - Re: [PATCH v2] char: tty3270: fix a missing check on list iterator

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]

Date:   Mon, 28 Mar 2022 10:47:00 +0200
From:   Sven Schnelle <svens@...ux.ibm.com>
To:     Xiaomeng Tong <xiam0nd.tong@...il.com>
Cc:     hca@...ux.ibm.com, gor@...ux.ibm.com, agordeev@...ux.ibm.com,
        borntraeger@...ux.ibm.com, gregkh@...uxfoundation.org,
        jirislaby@...nel.org, jcmvbkbc@...il.com, elder@...aro.org,
        dsterba@...e.com, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2] char: tty3270: fix a missing check on list iterator

Xiaomeng Tong <xiam0nd.tong@...il.com> writes:

> --- a/drivers/s390/char/tty3270.c
> +++ b/drivers/s390/char/tty3270.c
> @@ -1111,7 +1111,7 @@ tty3270_convert_line(struct tty3270 *tp, int line_nr)
>  {
>  	struct tty3270_line *line;
>  	struct tty3270_cell *cell;
> -	struct string *s, *n;
> +	struct string *s = NULL, *n, *iter;

Please keep reverse XMAS-tree layout.

>  	unsigned char highlight;
>  	unsigned char f_color;
>  	char *cp;
> @@ -1142,13 +1142,20 @@ tty3270_convert_line(struct tty3270 *tp, int line_nr)
>  
>  	/* Find the line in the list. */
>  	i = tp->view.rows - 2 - line_nr;
> -	list_for_each_entry_reverse(s, &tp->lines, list)
> -		if (--i <= 0)
> +	list_for_each_entry_reverse(iter, &tp->lines, list)
> +		if (--i <= 0) {
> +			s = iter;
>  			break;
> +		}
>  	/*
>  	 * Check if the line needs to get reallocated.
>  	 */
> -	if (s->len != flen) {
> +	if (!s) {
> +		/* Reallocate string. */
> +		n = tty3270_alloc_string(tp, flen);
> +		list_add(&n->list, &tp->lines);
> +		s = n;
> +	} else if (s->len != flen) {
>  		/* Reallocate string. */
>  		n = tty3270_alloc_string(tp, flen);
>  		list_add(&n->list, &s->list);

I should have written that in my first reply, but s == NULL means
the given line number couldn't be found in the list of lines. This is
a serious error and should be warned about. So maybe something like:

if (WARN_ON(!s))
	return;

But allocating a new empty line in that case is certainly wrong.

Thanks
Sven