Messages in this thread | | | Date | Mon, 16 Oct 2023 07:35:02 +0200 | Subject | Re: [PATCH v2 1/3] serial: core: Potential overflow of frame_time | From | Jiri Slaby <> |
| |
On 14. 10. 23, 20:13, Vamshi Gajjela wrote: > From: VAMSHI GAJJELA <vamshigajjela@google.com> > > uart_update_timeout() sets a u64 value to an unsigned int frame_time. > While it can be cast to u32 before assignment, there's a specific case > where frame_time is cast to u64. Since frame_time consistently > participates in u64 arithmetic, its data type is converted to u64 to > eliminate the need for explicit casting.
And make the divisions by the order of magnutude slower for no good reason? (Hint: have you looked what DIV64_U64_ROUND_UP() looks like on 32bit yet?)
Unless you provide a reason it can overflow in real (in fact you spell the opposite in the text above): NACKED-by: Jiri Slaby <jirislaby@kernel.org>
> Signed-off-by: VAMSHI GAJJELA <vamshigajjela@google.com> > --- > v2: > - use DIV64_U64_ROUND_UP with frame_time > > drivers/tty/serial/8250/8250_port.c | 2 +- > include/linux/serial_core.h | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c > index 141627370aab..d1bf794498c4 100644 > --- a/drivers/tty/serial/8250/8250_port.c > +++ b/drivers/tty/serial/8250/8250_port.c > @@ -1510,7 +1510,7 @@ static inline void __stop_tx(struct uart_8250_port *p) > * rather than after it is fully sent. > * Roughly estimate 1 extra bit here with / 7. > */ > - stop_delay = p->port.frame_time + DIV_ROUND_UP(p->port.frame_time, 7); > + stop_delay = p->port.frame_time + DIV64_U64_ROUND_UP(p->port.frame_time, 7); > } > > __stop_tx_rs485(p, stop_delay); > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h > index bb6f073bc159..b128513b009a 100644 > --- a/include/linux/serial_core.h > +++ b/include/linux/serial_core.h > @@ -558,7 +558,7 @@ struct uart_port { > > bool hw_stopped; /* sw-assisted CTS flow state */ > unsigned int mctrl; /* current modem ctrl settings */ > - unsigned int frame_time; /* frame timing in ns */ > + unsigned long frame_time; /* frame timing in ns */ > unsigned int type; /* port type */ > const struct uart_ops *ops; > unsigned int custom_divisor; > @@ -764,7 +764,7 @@ unsigned int uart_get_divisor(struct uart_port *port, unsigned int baud); > */ > static inline unsigned long uart_fifo_timeout(struct uart_port *port) > { > - u64 fifo_timeout = (u64)READ_ONCE(port->frame_time) * port->fifosize; > + u64 fifo_timeout = READ_ONCE(port->frame_time) * port->fifosize; > > /* Add .02 seconds of slop */ > fifo_timeout += 20 * NSEC_PER_MSEC;
-- js suse labs
| |