linux-kernel - RE: [PATCH 1/2] tty: serial: fsl_lpuart: fix the potential bug of division or modulo by zero

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: <AM0PR04MB49470E50CFAB8C36EABB241192429@AM0PR04MB4947.eurprd04.prod.outlook.com>
Date:   Mon, 26 Apr 2021 11:51:39 +0000
From:   Sherry Sun <sherry.sun@....com>
To:     Greg KH <gregkh@...uxfoundation.org>
CC:     "jirislaby@...nel.org" <jirislaby@...nel.org>,
        "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH 1/2] tty: serial: fsl_lpuart: fix the potential bug of
 division or modulo by zero

Hi Greg,

> > > >  drivers/tty/serial/fsl_lpuart.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > > > b/drivers/tty/serial/fsl_lpuart.c index 794035041744..777d54b593f8
> > > > 100644
> > > > --- a/drivers/tty/serial/fsl_lpuart.c
> > > > +++ b/drivers/tty/serial/fsl_lpuart.c
> > > > @@ -2414,6 +2414,9 @@ lpuart32_console_get_options(struct
> > > > lpuart_port *sport, int *baud,
> > > >
> > > >  	bd = lpuart32_read(&sport->port, UARTBAUD);
> > > >  	bd &= UARTBAUD_SBR_MASK;
> > > > +	if (!bd)
> > > > +		return;
> > >
> > > How can this ever happen?
> > >
> > > Not to say this is a bad check, but it feels like this can't really
> > > happen in real life, what code patch could create this result?
> > >
> > > And have you tested this on real hardware?
> > >
> >
> > Thanks for the reviewing, yes, I have tested the patchset on the real
> hardware.
> >
> > Seems the coverity check is static scan, so cannot judge if UARTBAUD
> Register will be zero.
> > I just found below statement in the uart reference manual: "When SBR is 1
> - 8191, the baud rate equals "baud clock / ((OSR+1) × SBR)"."
> > Since I am not familiar with uart, do you mean that the value of UARTBAUD
> Register will never be zero, so this case will not happen in real word?
> 
> Given that this never has happened with hardware for such an old device,
> perhaps it is impossible.  But it would be good to check.
> 
> > If yes, I will drop this patch.
> 
> Handling "bad data" from hardware is never a bad idea, so I don't
> necessarily want to drop this patch, I just want to try to figure out if this is a
> "incase the hardware is broken/malicious" type of change, vs.
> a "this bug we are seeing in real hardware" type of change.
> 

Yes, you are right, the probability of hardware happen in this case is really low. But we cannot guarantee that it will never happen.
So will this check here be accepted? Thanks!

Best regards
Sherry

> thanks,
> 
> greg k-h