linux-kernel - Re: [PATCH clk] clk: clkdev: add error messages for name exceeding maximum length

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] [day] [month] [year] [list]

Message-ID: <ZfF7QE0Jqok2uKrL@shell.armlinux.org.uk>
Date: Wed, 13 Mar 2024 10:09:04 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Duanqiang Wen <duanqiangwen@...-swift.com>
Cc: mturquette@...libre.com, sboyd@...nel.org, linux-clk@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH clk] clk: clkdev: add error messages for name exceeding
 maximum length

On Wed, Mar 13, 2024 at 02:42:52PM +0800, Duanqiang Wen wrote:
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index ee37d0be6877..620dc1e80b48 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -158,6 +158,9 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt,
>  	va_list ap)
>  {
>  	struct clk_lookup_alloc *cla;
> +	struct device *dev;
> +
> +	dev = clk_hw_get_dev(hw);

Sorry, but no, clkdev should have minimal dependencies on CCF (it was
designed to be completely independent). I'd prefer not to add this.
Just print the formatted dev_fmt+ap and the con_id when reporting
errors.

>  
>  	cla = kzalloc(sizeof(*cla), GFP_KERNEL);
>  	if (!cla)
> @@ -165,11 +168,19 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt,
>  
>  	cla->cl.clk_hw = hw;
>  	if (con_id) {
> +		if (strlen(dev_fmt) >= MAX_CON_ID) {

This is wrong (uses dev_fmt not con_id). Also, use sizeof(cla->con_id)
to test against.

> +			pr_err("%s:con_id string cannot be greater than 16 characters\n", dev_fmt);

Cleanup?

> +			return NULL;
> +		}
>  		strscpy(cla->con_id, con_id, sizeof(cla->con_id));
>  		cla->cl.con_id = cla->con_id;
>  	}
>  
>  	if (dev_fmt) {
> +		if (strlen(dev_fmt) >= MAX_DEV_ID) {

This is also wrong. The length of the format string does not give any
information on how long the resulting string actually is.

> +			pr_err("%s:dev_id string cannot be greater than 20 characters\n", dev_fmt);

Cleanup?

> +			return NULL;
> +		}
>  		vscnprintf(cla->dev_id, sizeof(cla->dev_id), dev_fmt, ap);

Using vsnprintf() here and checking whether the return value is larger
than sizeof(cla->dev_id) would be better.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!