From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1fIT7b-00034s-Rl for barebox@lists.infradead.org; Tue, 15 May 2018 06:07:42 +0000 Date: Tue, 15 May 2018 08:07:27 +0200 From: Sascha Hauer Message-ID: <20180515060727.j6zcpqcu7vwbd3bn@pengutronix.de> References: <20180508211502.5476-1-andrew.smirnov@gmail.com> <20180514071456.qknyha5bsl6gbkk4@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "barebox" Errors-To: barebox-bounces+u.kleine-koenig=pengutronix.de@lists.infradead.org Subject: Re: [PATCH] serial: lpuart: Avoid division by zero when requested baudrate is To: Andrey Smirnov Cc: Barebox List On Mon, May 14, 2018 at 05:20:56PM -0700, Andrey Smirnov wrote: > On Mon, May 14, 2018 at 12:14 AM, Sascha Hauer wrote: > > On Tue, May 08, 2018 at 02:15:02PM -0700, Andrey Smirnov wrote: > >> With serdev device support added there's now a corner case where: > >> > >> 1. There is a DT node for a serdev device on one of the UARTs > >> 2. There is no driver that binds against serdev device's compatibility > >> string > >> > >> with 1 and 2 being true it is possible to end up in a situation where > >> a particualr UART has not been initalized to any baudrate when > >> clock_notifier_call_chain() gets called. This effectively translates > >> to > >> > >> set_baudrate(uart, 0); > >> > >> which for LPUART driver result in a division by zero. > > > > This probably leads to a division by zero for most drivers since > > dividing by the baudrate is a common pattern. Wouldn't it be better to > > catch this is console_set_baudrate()? > > > > It would, but none of the drivers that register a callback with > clock_register_client() (serial_imx.c, serial_auart.c, stm-serial.c, > serial_lpuart.c and serial_cadence.c) use console_set_badrate() and > instead call ->setbrg() directly. Probably because > console_set_baudrate() also handles interactive baudrate switch for > f_active console devices. Serial_imx.c doesn't preform any division, > so it is safe and I can go through the rest of them to add appropriate > checks? Another option is probably to try to come up with common > clock_register_client() infrastructure(since all of the handlers are > the same) and keep that check there. > > Let me know if you want me to do either. I see. For the sake of simplicity I think we can go with your original patch. In the auart and cadence driver the clock notify stuff is unused, only copy paste leftover from copying the i.MX driver. That leaves the lpuart driver which really has this problem. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox