linux-kernel - Re: [PATCH 03/13] serial: don't use uninitialized value in uart_poll

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: <CAD=FV=X1a1a=kkJ9bWXWOmu0hz6HqRuK=Vo=bhvFfSzeAWSWyw@mail.gmail.com>
Date: Mon, 5 Aug 2024 08:43:48 -0700
From: Doug Anderson <dianders@...omium.org>
To: "Jiri Slaby (SUSE)" <jirislaby@...nel.org>
Cc: gregkh@...uxfoundation.org, linux-serial@...r.kernel.org, 
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 03/13] serial: don't use uninitialized value in uart_poll_init()

Hi,

On Mon, Aug 5, 2024 at 3:21 AM Jiri Slaby (SUSE) <jirislaby@...nel.org> wrote:
>
> Coverity reports (as CID 1536978) that uart_poll_init() passes
> uninitialized pm_state to uart_change_pm(). It is in case the first 'if'
> takes the true branch (does "goto out;").
>
> Fix this and simplify the function by simple guard(mutex). The code
> needs no labels after this at all. And it is pretty clear that the code
> has not fiddled with pm_state at that point.
>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@...nel.org>
> Fixes: 5e227ef2aa38 (serial: uart_poll_init() should power on the UART)
> Cc: stable@...r.kernel.org
> Cc: Douglas Anderson <dianders@...omium.org>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
>  drivers/tty/serial/serial_core.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)

Thanks for the fix! Looks good.

Reviewed-by: Douglas Anderson <dianders@...omium.org>

NOTE: I'm happy to defer to others, but personally I'd consider
breaking this into two changes: one that fixes the problem without
using guard() (which should be pretty simple) and one that switches to
guard(). The issue is that at the time the bug was introduced the
guard() syntax didn't exist and that means backporting will be a bit
of a pain.

Oh, though I guess maybe it doesn't matter since the bug was
introduced in 6.4 and that's not an LTS kernel so nobody cares? ...and
guard() is in 6.6, so maybe things are fine the way you have it.