From linux-kernel Fri Jun 09 08:57:14 2023 From: Walter Harms Date: Fri, 09 Jun 2023 08:57:14 +0000 To: linux-kernel Subject: AW: [PATCH 2/2] tty: serial: samsung_tty: Fix a memory leak in s3c24xx_serial_getclk() when iteratin Message-Id: X-MARC-Message: https://marc.info/?l=linux-kernel&m=168631472803049 while we are here .... perhaps INT_MAX from kernel.h ? int deviation =3D (1 << 30) - 1; the part before looks a bit strange if (ourport->info->has_divslot) { unsigned long div =3D rate / req_baud; /* The UDIVSLOT register on the newer UARTs allows = us to * get a divisor adjustment of 1/16th on the baud c= lock. * * We don't keep the UDIVSLOT value (the 16ths we * calculated by not multiplying the baud by 16) as= it * is easy enough to recalculate. */ quot =3D div / 16; baud =3D rate / div; because baud=3Drate/rate/req_baud =3D req_baud can this be simplyfied ? (or is the numeric required ?) Homebrew abs() kernel.h has a abs() can we use it here ? if (calc_deviation < 0) calc_deviation =3D -calc_deviation; to the patch: + /* + * If we find a better clk, release the previous on= e, if + * any. + */ + if (!IS_ERR(*best_clk)) + clk_put(*best_clk); the intentions are good. *best_clk is user supplied (and should be NULL) filled & released in the next round but IMHO must be valid (is clk). so no need to check. (ntl clk_put seems to handle NULL and ERR ) if (!clk || WARN_ON_ONCE(IS_ERR(clk))) return; JM2C wh ________________________________________ Von: Christophe JAILLET Gesendet: Freitag, 9. Juni 2023 06:45:39 An: Krzysztof Kozlowski; Alim Akhtar; Greg Kroah-Hartman; Jiri Slaby; Thoma= s Abraham; Kukjin Kim Cc: linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org; Christop= he JAILLET; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.ke= rnel.org; linux-serial@vger.kernel.org Betreff: [PATCH 2/2] tty: serial: samsung_tty: Fix a memory leak in s3c24xx= _serial_getclk() when iterating clk When the best clk is searched, we iterate over all possible clk. If we find a better match, the previous one, if any, needs to be freed. If a better match has already been found, we still need to free the new one, otherwise it leaks. Fixes: 5f5a7a5578c5 ("serial: samsung: switch to clkdev based clock lookup"= ) Signed-off-by: Christophe JAILLET --- This patch is speculative. Review with care. I think that some clk_put() are also missing somewhere else in the driver but won't be able to investigate further. --- drivers/tty/serial/samsung_tty.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_= tty.c index dd751e7010e3..c07877dd25fa 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -1488,10 +1488,18 @@ static unsigned int s3c24xx_serial_getclk(struct s3= c24xx_uart_port *ourport, calc_deviation =3D -calc_deviation; if (calc_deviation < deviation) { + /* + * If we find a better clk, release the previous on= e, if + * any. + */ + if (!IS_ERR(*best_clk)) + clk_put(*best_clk); *best_clk =3D clk; best_quot =3D quot; *clk_num =3D cnt; deviation =3D calc_deviation; + } else { + clk_put(clk); } } -- 2.34.1