| Message ID | 20240425-cbl-bcm-assign-counted-by-val-before-access-v1-2-e2db3b82d5ef@kernel.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | clk: bcm: Move a couple of __counted_by initializations | expand |
On Thu, Apr 25, 2024 at 09:55:52AM -0700, Nathan Chancellor wrote: > Commit f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with > __counted_by") annotated the hws member of 'struct clk_hw_onecell_data' > with __counted_by, which informs the bounds sanitizer about the number > of elements in hws, so that it can warn when hws is accessed out of > bounds. As noted in that change, the __counted_by member must be > initialized with the number of elements before the first array access > happens, otherwise there will be a warning from each access prior to the > initialization because the number of elements is zero. This occurs in > raspberrypi_discover_clocks() due to ->num being assigned after ->hws > has been accessed: > > UBSAN: array-index-out-of-bounds in drivers/clk/bcm/clk-raspberrypi.c:374:4 > index 3 is out of range for type 'struct clk_hw *[] __counted_by(num)' (aka 'struct clk_hw *[]') > > Move the ->num initialization to before the first access of ->hws, which > clears up the warning. > > Cc: stable@vger.kernel.org > Fixes: f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with __counted_by") > Signed-off-by: Nathan Chancellor <nathan@kernel.org> Looks good; thanks! Reviewed-by: Kees Cook <keescook@chromium.org>
On 4/25/24 09:55, Nathan Chancellor wrote: > Commit f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with > __counted_by") annotated the hws member of 'struct clk_hw_onecell_data' > with __counted_by, which informs the bounds sanitizer about the number > of elements in hws, so that it can warn when hws is accessed out of > bounds. As noted in that change, the __counted_by member must be > initialized with the number of elements before the first array access > happens, otherwise there will be a warning from each access prior to the > initialization because the number of elements is zero. This occurs in > raspberrypi_discover_clocks() due to ->num being assigned after ->hws > has been accessed: > > UBSAN: array-index-out-of-bounds in drivers/clk/bcm/clk-raspberrypi.c:374:4 > index 3 is out of range for type 'struct clk_hw *[] __counted_by(num)' (aka 'struct clk_hw *[]') > > Move the ->num initialization to before the first access of ->hws, which > clears up the warning. > > Cc: stable@vger.kernel.org > Fixes: f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with __counted_by") > Signed-off-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Quoting Nathan Chancellor (2024-04-25 09:55:52) > Commit f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with > __counted_by") annotated the hws member of 'struct clk_hw_onecell_data' > with __counted_by, which informs the bounds sanitizer about the number > of elements in hws, so that it can warn when hws is accessed out of > bounds. As noted in that change, the __counted_by member must be > initialized with the number of elements before the first array access > happens, otherwise there will be a warning from each access prior to the > initialization because the number of elements is zero. This occurs in > raspberrypi_discover_clocks() due to ->num being assigned after ->hws > has been accessed: > > UBSAN: array-index-out-of-bounds in drivers/clk/bcm/clk-raspberrypi.c:374:4 > index 3 is out of range for type 'struct clk_hw *[] __counted_by(num)' (aka 'struct clk_hw *[]') > > Move the ->num initialization to before the first access of ->hws, which > clears up the warning. > > Cc: stable@vger.kernel.org > Fixes: f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with __counted_by") > Signed-off-by: Nathan Chancellor <nathan@kernel.org> > --- Applied to clk-next
diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c index 829406dc44a2..4d411408e4af 100644 --- a/drivers/clk/bcm/clk-raspberrypi.c +++ b/drivers/clk/bcm/clk-raspberrypi.c @@ -371,8 +371,8 @@ static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi, if (IS_ERR(hw)) return PTR_ERR(hw); - data->hws[clks->id] = hw; data->num = clks->id + 1; + data->hws[clks->id] = hw; } clks++;
Commit f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with __counted_by") annotated the hws member of 'struct clk_hw_onecell_data' with __counted_by, which informs the bounds sanitizer about the number of elements in hws, so that it can warn when hws is accessed out of bounds. As noted in that change, the __counted_by member must be initialized with the number of elements before the first array access happens, otherwise there will be a warning from each access prior to the initialization because the number of elements is zero. This occurs in raspberrypi_discover_clocks() due to ->num being assigned after ->hws has been accessed: UBSAN: array-index-out-of-bounds in drivers/clk/bcm/clk-raspberrypi.c:374:4 index 3 is out of range for type 'struct clk_hw *[] __counted_by(num)' (aka 'struct clk_hw *[]') Move the ->num initialization to before the first access of ->hws, which clears up the warning. Cc: stable@vger.kernel.org Fixes: f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with __counted_by") Signed-off-by: Nathan Chancellor <nathan@kernel.org> --- drivers/clk/bcm/clk-raspberrypi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)