| Message ID | YyMM8iVSHJ4ammsg@kili |
|---|---|
| State | Accepted |
| Commit | b7af938f4379a884f15713319648a7653497a907 |
| Headers | show |
| Series | i2c: mux: harden i2c_mux_alloc() against integer overflows | expand |
On Fri, Sep 16, 2022 at 05:55:55PM +0300, Dan Carpenter wrote: > On Fri, Sep 16, 2022 at 06:31:45AM -0700, Kees Cook wrote: > > On Fri, Sep 16, 2022 at 11:23:25AM +0300, Dan Carpenter wrote: > > > [...] > > > net/ipv6/mcast.c:450 ip6_mc_source() saving 'size_add' to type 'int' > > > > Interesting! Are you able to report the consumer? e.g. I think a bunch > > of these would be fixed by: > > > > Are you asking if I can add "passed to sock_kmalloc()" to the report? Yeah. > It's possible but it's kind of a headache the way this code is written. Okay, no worries -- I was curious if it would be "easy". I can happily just spit out the source line.
On Fri, Sep 16, 2022 at 08:31:58PM +0100, Wolfram Sang wrote: > > > > The new variable makes it more readable, but beyond that, do you see any > > > reason not to just directly compose the calls? > > > > > > > You could do that too. > > > > You pointed this out in your other email but the one thing that people > > have to be careful of when assigning struct_size() is that the > > "mux_size" variable has to be size_t. > > > > The math in submit_create() from drivers/gpu/drm/msm/msm_gem_submit.c > > is so terribly unreadable. It works but it's so ugly. Unfortunately, > > I'm the person who wrote it. > > I can't parse from that if the patch in question is okay or needs a > respin? Could you kindly enlighten me? > It doesn't need a respin. We were just discussing related bugs with the integer overflow safe functions. regards, dan carpenter
On Thu, Sep 15, 2022 at 02:30:58PM +0300, Dan Carpenter wrote: > A couple years back we went through the kernel an automatically > converted size calculations to use struct_size() instead. The > struct_size() calculation is protected against integer overflows. > > However it does not make sense to use the result from struct_size() > for additional math operations as that would negate any safeness. > > Fixes: 1f3b69b6b939 ("i2c: mux: Use struct_size() in devm_kzalloc()") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Applied to for-current, thanks!
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c index 774507b54b57..313904be5f3b 100644 --- a/drivers/i2c/i2c-mux.c +++ b/drivers/i2c/i2c-mux.c @@ -243,9 +243,10 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent, int (*deselect)(struct i2c_mux_core *, u32)) { struct i2c_mux_core *muxc; + size_t mux_size; - muxc = devm_kzalloc(dev, struct_size(muxc, adapter, max_adapters) - + sizeof_priv, GFP_KERNEL); + mux_size = struct_size(muxc, adapter, max_adapters); + muxc = devm_kzalloc(dev, size_add(mux_size, sizeof_priv), GFP_KERNEL); if (!muxc) return NULL; if (sizeof_priv)
A couple years back we went through the kernel an automatically converted size calculations to use struct_size() instead. The struct_size() calculation is protected against integer overflows. However it does not make sense to use the result from struct_size() for additional math operations as that would negate any safeness. Fixes: 1f3b69b6b939 ("i2c: mux: Use struct_size() in devm_kzalloc()") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/i2c/i2c-mux.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)