RE: [PATCH] tty: serial: fsl_lpuart: Fix possible integer overflow

[Sherry Sun]

> -----Original Message-----
> From: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> Sent: 2023年7月18日 17:56
> To: Sherry Sun <sherry.sun@xxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Shenwei Wang
> <shenwei.wang@xxxxxxx>; linux-serial <linux-serial@xxxxxxxxxxxxxxx>;
> LKML <linux-kernel@xxxxxxxxxxxxxxx>; dl-linux-imx <linux-imx@xxxxxxx>;
> Jiri Slaby <jirislaby@xxxxxxxxxx>
> Subject: Re: [PATCH] tty: serial: fsl_lpuart: Fix possible integer overflow
> 
> On Tue, 18 Jul 2023, Jiri Slaby wrote:
> 
> > On 18. 07. 23, 8:56, Sherry Sun wrote:
> > > This patch addresses the following Coverity report, fix it by
> > > casting
> > > sport->port.frame_time to type u64.
> > >
> > > CID 32305660: Unintentional integer overflow
> (OVERFLOW_BEFORE_WIDEN)
> > > Potentially overflowing expression sport->port.frame_time * 8U with
> > > type unsigned int (32 bits, unsigned) is evaluated using 32-bit
> > > arithmetic, and then used in a context that expects an expression of
> > > type u64 (64 bits, unsigned).
> > >
> > > Fixes: cf9aa72d2f91 ("tty: serial: fsl_lpuart: optimize the timer
> > > based EOP
> > > logic")
> > > Signed-off-by: Sherry Sun <sherry.sun@xxxxxxx>
> > > ---
> > >   drivers/tty/serial/fsl_lpuart.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > > b/drivers/tty/serial/fsl_lpuart.c index c1980ea52666..07b3b26732db
> > > 100644
> > > --- a/drivers/tty/serial/fsl_lpuart.c
> > > +++ b/drivers/tty/serial/fsl_lpuart.c
> > > @@ -1373,7 +1373,7 @@ static inline int lpuart_start_rx_dma(struct
> > > lpuart_port *sport)
> > >     	sport->last_residue = 0;
> > >   	sport->dma_rx_timeout = max(nsecs_to_jiffies(
> > > -		sport->port.frame_time * DMA_RX_IDLE_CHARS), 1UL);
> > > +		(u64)sport->port.frame_time * DMA_RX_IDLE_CHARS), 1UL);
> >
> > Can you explain how that can overflow? In the worst case (1 start bit,
> > 8 data bits, 2 stop bits, parity bit, address bit, 50 bauds),
> > frame_time would
> > contain:
> > 13*1e9/50 = 260,000,000. (260 ms)
> >
> > Then the multiplication above is:
> > 260,000,000*8 = 2,080,000,000. (2 seconds)
> >
> > which is still less than 2^32-1.
> 
> I was wondering the same thing.
> 
> This isn't a real bug. All findings from code analysis tools must be carefully
> evaluated to filter wheat out of chaff and this falls into the latter category.
> Please make sure next time you understand and explain also in the
> changelog how the problem can be manifested for real before sending this
> kind of patches.
> 

Hi Ilpo and Jiri,
You are right, now the DMA_RX_IDLE_CHARS is 8, so even the worst case frametime won't overflow uint32.
Thanks for the reminder, I will drop the patch and pay attention next time.

Best Regards
Sherry