linux-toradex.git/drivers/clk/visconti, branch v6.11-rc3

clk: visconti: Add bounds-checking coverage for struct visconti_pll_provider

2023-10-24T02:40:20+00:00

In order to gain the bounds-checking coverage that __counted_by provides
to flexible-array members at run-time via CONFIG_UBSAN_BOUNDS (for array
indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions),
we must make sure that the counter member, in this particular case `num`,
is updated before the first access to the flex-array member, in this
particular case array `hws`. See below:

commit f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with
__counted_by") introduced `__counted_by` for `struct clk_hw_onecell_data`
together with changes to relocate some of assignments of counter `num`
before `hws` is accessed:

include/linux/clk-provider.h:
1380 struct clk_hw_onecell_data {
1381         unsigned int num;
1382         struct clk_hw *hws[] __counted_by(num);
1383 };

However, this structure is used as a member in other structs, in this
case in `struct visconti_pll_provider`:

drivers/clk/visconti/pll.h:
 16 struct visconti_pll_provider {
 17         void __iomem *reg_base;
 18         struct device_node *node;
 19
 20         /* Must be last */
 21         struct clk_hw_onecell_data clk_data;
 22 };

Hence, we need to move the assignments to `ctx->clk_data.num` after
allocation for `struct visconti_pll_provider` and before accessing the
flexible array `ctx->clk_data.hws`. And, as assignments for all members
in `struct visconti_pll_provider` are originally adjacent to each other,
relocate all assignments together, so we don't split up
`ctx->clk_data.hws = nr_plls` from the rest. :)

Reviewed-by: Kees Cook 
Acked-by: Nobuhiro Iwamatsu 
Signed-off-by: Gustavo A. R. Silva 
Link: https://lore.kernel.org/r/e3189f3e40e8723b6d794fb2260e2e9ab6b960bd.1697492890.git.gustavoars@kernel.org
Signed-off-by: Stephen Boyd

clk: visconti: Fix undefined behavior bug in struct visconti_pll_provider

2023-10-24T02:40:20+00:00

`struct clk_hw_onecell_data` is a flexible structure, which means that
it contains flexible-array member at the bottom, in this case array
`hws`:

include/linux/clk-provider.h:
1380 struct clk_hw_onecell_data {
1381         unsigned int num;
1382         struct clk_hw *hws[] __counted_by(num);
1383 };

This could potentially lead to an overwrite of the objects following
`clk_data` in `struct visconti_pll_provider`, in this case
`struct device_node *node;`, at run-time:

drivers/clk/visconti/pll.h:
 16 struct visconti_pll_provider {
 17         void __iomem *reg_base;
 18         struct clk_hw_onecell_data clk_data;
 19         struct device_node *node;
 20 };

Notice that a total of 56 bytes are allocated for flexible-array `hws`
at line 328. See below:

include/dt-bindings/clock/toshiba,tmpv770x.h:
 14 #define TMPV770X_NR_PLL		7

drivers/clk/visconti/pll-tmpv770x.c:
 69 ctx = visconti_init_pll(np, reg_base, TMPV770X_NR_PLL);

drivers/clk/visconti/pll.c:
321 struct visconti_pll_provider * __init visconti_init_pll(struct device_node *np,
322                                                         void __iomem *base,
323                                                         unsigned long nr_plls)
324 {
325         struct visconti_pll_provider *ctx;
...
328         ctx = kzalloc(struct_size(ctx, clk_data.hws, nr_plls), GFP_KERNEL);

`struct_size(ctx, clk_data.hws, nr_plls)` above translates to
sizeof(struct visconti_pll_provider) + sizeof(struct clk_hw *) * 7 ==
24 + 8 * 7 == 24 + 56
		  ^^^^
		   |
	allocated bytes for flex array `hws`

$ pahole -C visconti_pll_provider drivers/clk/visconti/pll.o
struct visconti_pll_provider {
	void *                     reg_base;             /*     0     8 */
	struct clk_hw_onecell_data clk_data;             /*     8     8 */
	struct device_node *       node;                 /*    16     8 */

	/* size: 24, cachelines: 1, members: 3 */
	/* last cacheline: 24 bytes */
};

And then, after the allocation, some data is written into all members
of `struct visconti_pll_provider`:

332         for (i = 0; i < nr_plls; ++i)
333                 ctx->clk_data.hws[i] = ERR_PTR(-ENOENT);
334
335         ctx->node = np;
336         ctx->reg_base = base;
337         ctx->clk_data.num = nr_plls;

Fix all these by placing the declaration of object `clk_data` at the
end of `struct visconti_pll_provider`. Also, add a comment to make it
clear that this object must always be last in the structure, and
prevent this bug from being introduced again in the future.

-Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
ready to enable it globally.

Fixes: b4cbe606dc36 ("clk: visconti: Add support common clock driver and reset driver")
Cc: stable@vger.kernel.org
Reviewed-by: Kees Cook 
Acked-by: Nobuhiro Iwamatsu 
Signed-off-by: Gustavo A. R. Silva 
Link: https://lore.kernel.org/r/57a831d94ee2b3889b11525d4ad500356f89576f.1697492890.git.gustavoars@kernel.org
Signed-off-by: Stephen Boyd

clk: visconti: remove unused visconti_pll_provider::regmap

2023-03-06T19:43:29+00:00

Field regmap of struct visconti_pll_provider is never used. Remove it.

Signed-off-by: Dario Binacchi 

Link: https://lore.kernel.org/r/20230302205028.2539197-1-dario.binacchi@amarulasolutions.com
Acked-by: Nobuhiro Iwamatsu 
Signed-off-by: Stephen Boyd

clk: visconti: Fix memory leak in visconti_register_pll()

2022-11-29T20:11:00+00:00

@pll->rate_table has allocated memory by kmemdup(), if clk_hw_register()
fails, it should be freed, otherwise it will cause memory leak issue,
this patch fixes it.

Fixes: b4cbe606dc36 ("clk: visconti: Add support common clock driver and reset driver")
Signed-off-by: Xiu Jianfeng 
Link: https://lore.kernel.org/r/20221122152353.204132-1-xiujianfeng@huawei.com
Acked-by: Nobuhiro Iwamatsu 
Signed-off-by: Stephen Boyd

clk: visconti: prevent array overflow in visconti_clk_register_gates()

2022-03-17T19:21:28+00:00

This code was using -1 to represent that there was no reset function.
Unfortunately, the -1 was stored in u8 so the if (clks[i].rs_id >= 0)
condition was always true.  This lead to an out of bounds access in
visconti_clk_register_gates().

Fixes: b4cbe606dc36 ("clk: visconti: Add support common clock driver and reset driver")
Signed-off-by: Dan Carpenter 
Link: https://lore.kernel.org/r/20220316083533.GA30941@kili
Acked-by: Nobuhiro Iwamatsu 
Signed-off-by: Stephen Boyd

clk: visconti: Fix uninitialized variable in printk

2022-01-13T21:21:02+00:00

The "pll_clck" variable is uninitialized.  The "ret" error code was
supposed to be printed instead.

Fixes: b4cbe606dc36 ("clk: visconti: Add support common clock driver and reset driver")
Signed-off-by: Dan Carpenter 
Link: https://lore.kernel.org/r/20220111072529.GJ11243@kili
Signed-off-by: Stephen Boyd

clk: visconti: Remove pointless NULL check in visconti_pll_add_lookup()

2022-01-08T00:08:07+00:00

Clang warns:

drivers/clk/visconti/pll.c:292:20: warning: address of array 'ctx->clk_data.hws' will always evaluate to 'true' [-Wpointer-bool-conversion]
        if (ctx->clk_data.hws && id)
            ~~~~~~~~~~~~~~^~~ ~~
1 warning generated.

This array cannot be NULL if ctx is not NULL, which is allocated in
visconti_init_pll(), so just remove the check, which matches other clk
drivers.

Fixes: b4cbe606dc36 ("clk: visconti: Add support common clock driver and reset driver")
Link: https://github.com/ClangBuiltLinux/linux/issues/1564
Reported-by: kernel test robot 
Signed-off-by: Nathan Chancellor 
Link: https://lore.kernel.org/r/20220107183303.2337676-1-nathan@kernel.org
Signed-off-by: Stephen Boyd

clk: visconti: Add support common clock driver and reset driver

2022-01-06T01:12:31+00:00

Add support for common interface of the common clock and reset driver
for Toshiba Visconti5 and its SoC, TMPV7708. The PIPLLCT provides the PLL,
and the PISMU provides clock and reset functionality.
Each drivers are provided in this patch.

Signed-off-by: Nobuhiro Iwamatsu 
Link: https://lore.kernel.org/r/20211025031038.4180686-4-nobuhiro1.iwamatsu@toshiba.co.jp
[sboyd@kernel.org: Add bitfield.h include to pll.c]
Signed-off-by: Stephen Boyd