[RESEND][PATCH v2] clk_register_clkdev: remove format string interface
Kees Cook
keescook at chromium.org
Wed Jan 13 12:37:50 PST 2016
On Thu, Jan 7, 2016 at 11:32 AM, Kees Cook <keescook at chromium.org> wrote:
> Many callers either use NULL or const strings for the third argument of
> clk_register_clkdev. For those that do not and use a non-const string,
> this is a risk for format strings being accidentally processed (for
> example in device names). As this interface is already used as if it
> weren't a format string (prints nothing when NULL), and there are zero
> users of the format strings, remove the format string interface to make
> sure format strings will not leak into the clkdev.
>
> $ git grep '\bclk_register_clkdev\b' | grep % | wc -l
> 0
>
> Unfortunately, all the internals expect a va_list even though they treat
> a NULL format string as special. To deal with this, we must pass either
> (..., "%s", string) or (..., NULL) so that a the va_list will be created
> correctly (passing the name as an argument, not as a format string).
>
> Signed-off-by: Kees Cook <keescook at chromium.org>
> ---
> v2:
> - make calling logic more clear, thanks to rmk.
Should this go via -mm, a clk tree, or through the ARM patch tracker?
Thanks!
-Kees
> ---
> drivers/clk/clkdev.c | 31 +++++++++++++++++++++++++------
> include/linux/clkdev.h | 3 +--
> 2 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index 779b6ff0c7ad..eb20b941154b 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -353,11 +353,25 @@ void clkdev_drop(struct clk_lookup *cl)
> }
> EXPORT_SYMBOL(clkdev_drop);
>
> +static struct clk_lookup *__clk_register_clkdev(struct clk_hw *hw,
> + const char *con_id,
> + const char *dev_id, ...)
> +{
> + struct clk_lookup *cl;
> + va_list ap;
> +
> + va_start(ap, dev_id);
> + cl = vclkdev_create(hw, con_id, dev_id, ap);
> + va_end(ap);
> +
> + return cl;
> +}
> +
> /**
> * clk_register_clkdev - register one clock lookup for a struct clk
> * @clk: struct clk to associate with all clk_lookups
> * @con_id: connection ID string on device
> - * @dev_id: format string describing device name
> + * @dev_id: string describing device name
> *
> * con_id or dev_id may be NULL as a wildcard, just as in the rest of
> * clkdev.
> @@ -368,17 +382,22 @@ EXPORT_SYMBOL(clkdev_drop);
> * after clk_register().
> */
> int clk_register_clkdev(struct clk *clk, const char *con_id,
> - const char *dev_fmt, ...)
> + const char *dev_id)
> {
> struct clk_lookup *cl;
> - va_list ap;
>
> if (IS_ERR(clk))
> return PTR_ERR(clk);
>
> - va_start(ap, dev_fmt);
> - cl = vclkdev_create(__clk_get_hw(clk), con_id, dev_fmt, ap);
> - va_end(ap);
> + /*
> + * Since dev_id can be NULL, and NULL is handled specially, we must
> + * pass it as either a NULL format string, or with "%s".
> + */
> + if (dev_id)
> + cl = __clk_register_clkdev(__clk_get_hw(clk), con_id, "%s",
> + dev_id);
> + else
> + cl = __clk_register_clkdev(__clk_get_hw(clk), con_id, NULL);
>
> return cl ? 0 : -ENOMEM;
> }
> diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h
> index 08bffcc466de..c2c04f7cbe8a 100644
> --- a/include/linux/clkdev.h
> +++ b/include/linux/clkdev.h
> @@ -44,8 +44,7 @@ struct clk_lookup *clkdev_create(struct clk *clk, const char *con_id,
> void clkdev_add_table(struct clk_lookup *, size_t);
> int clk_add_alias(const char *, const char *, const char *, struct device *);
>
> -int clk_register_clkdev(struct clk *, const char *, const char *, ...)
> - __printf(3, 4);
> +int clk_register_clkdev(struct clk *, const char *, const char *);
> int clk_register_clkdevs(struct clk *, struct clk_lookup *, size_t);
>
> #ifdef CONFIG_COMMON_CLK
> --
> 2.6.3
>
>
> --
> Kees Cook
> Chrome OS & Brillo Security
--
Kees Cook
Chrome OS & Brillo Security
More information about the linux-arm-kernel
mailing list