| Message ID | 20190219170209.4180739-1-arnd@arndb.de |
|---|---|
| State | Accepted |
| Commit | 03aa4f191a36f33fce015387f84efa0eee94408e |
| Headers | show |
| Series | [1/3] media: saa7146: avoid high stack usage with clang | expand |
On Tue, Feb 19, 2019 at 9:02 AM Arnd Bergmann <arnd@arndb.de> wrote: > > Two saa7146/hexium files contain a construct that causes a warning > when built with clang: > > drivers/media/pci/saa7146/hexium_orion.c:210:12: error: stack frame size of 2272 bytes in function 'hexium_probe' > [-Werror,-Wframe-larger-than=] > static int hexium_probe(struct saa7146_dev *dev) > ^ > drivers/media/pci/saa7146/hexium_gemini.c:257:12: error: stack frame size of 2304 bytes in function 'hexium_attach' > [-Werror,-Wframe-larger-than=] > static int hexium_attach(struct saa7146_dev *dev, struct saa7146_pci_extension_data *info) > ^ > > This one happens regardless of KASAN, and the problem is that a > constructor to initalize a dynamically allocated structure leads > to a copy of that structure on the stack, whereas gcc initializes > it in place. > > Link: https://bugs.llvm.org/show_bug.cgi?id=40776 oof, great bug report by the way! Thanks for the fix. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/media/pci/saa7146/hexium_gemini.c | 4 +--- > drivers/media/pci/saa7146/hexium_orion.c | 4 +--- > 2 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/pci/saa7146/hexium_gemini.c b/drivers/media/pci/saa7146/hexium_gemini.c > index 5817d9cde4d0..f7ce0e1770bf 100644 > --- a/drivers/media/pci/saa7146/hexium_gemini.c > +++ b/drivers/media/pci/saa7146/hexium_gemini.c > @@ -270,9 +270,7 @@ static int hexium_attach(struct saa7146_dev *dev, struct saa7146_pci_extension_d > /* enable i2c-port pins */ > saa7146_write(dev, MC1, (MASK_08 | MASK_24 | MASK_10 | MASK_26)); > > - hexium->i2c_adapter = (struct i2c_adapter) { > - .name = "hexium gemini", > - }; > + strscpy(hexium->i2c_adapter.name, "hexium gemini", sizeof(hexium->i2c_adapter.name)); > saa7146_i2c_adapter_prepare(dev, &hexium->i2c_adapter, SAA7146_I2C_BUS_BIT_RATE_480); > if (i2c_add_adapter(&hexium->i2c_adapter) < 0) { > DEB_S("cannot register i2c-device. skipping.\n"); > diff --git a/drivers/media/pci/saa7146/hexium_orion.c b/drivers/media/pci/saa7146/hexium_orion.c > index 0a05176c18ab..b9f4a09c744d 100644 > --- a/drivers/media/pci/saa7146/hexium_orion.c > +++ b/drivers/media/pci/saa7146/hexium_orion.c > @@ -231,9 +231,7 @@ static int hexium_probe(struct saa7146_dev *dev) > saa7146_write(dev, DD1_STREAM_B, 0x00000000); > saa7146_write(dev, MC2, (MASK_09 | MASK_25 | MASK_10 | MASK_26)); > > - hexium->i2c_adapter = (struct i2c_adapter) { > - .name = "hexium orion", > - }; > + strscpy(hexium->i2c_adapter.name, "hexium orion", sizeof(hexium->i2c_adapter.name)); Note that "sparse" designated initialization zero initializes unnnamed members: https://godbolt.org/z/LkSpJp This transform you've done is safe because hexium was zero initialized via kzalloc, and struct hexium contains a struct i2c_adapter (as opposed to a pointer to a struct i2c_adapter). The same is true for both translation units you've touched. LGTM > saa7146_i2c_adapter_prepare(dev, &hexium->i2c_adapter, SAA7146_I2C_BUS_BIT_RATE_480); > if (i2c_add_adapter(&hexium->i2c_adapter) < 0) { > DEB_S("cannot register i2c-device. skipping.\n"); > -- > 2.20.0 > -- Thanks, ~Nick Desaulniers
diff --git a/drivers/media/pci/saa7146/hexium_gemini.c b/drivers/media/pci/saa7146/hexium_gemini.c index 5817d9cde4d0..f7ce0e1770bf 100644 --- a/drivers/media/pci/saa7146/hexium_gemini.c +++ b/drivers/media/pci/saa7146/hexium_gemini.c @@ -270,9 +270,7 @@ static int hexium_attach(struct saa7146_dev *dev, struct saa7146_pci_extension_d /* enable i2c-port pins */ saa7146_write(dev, MC1, (MASK_08 | MASK_24 | MASK_10 | MASK_26)); - hexium->i2c_adapter = (struct i2c_adapter) { - .name = "hexium gemini", - }; + strscpy(hexium->i2c_adapter.name, "hexium gemini", sizeof(hexium->i2c_adapter.name)); saa7146_i2c_adapter_prepare(dev, &hexium->i2c_adapter, SAA7146_I2C_BUS_BIT_RATE_480); if (i2c_add_adapter(&hexium->i2c_adapter) < 0) { DEB_S("cannot register i2c-device. skipping.\n"); diff --git a/drivers/media/pci/saa7146/hexium_orion.c b/drivers/media/pci/saa7146/hexium_orion.c index 0a05176c18ab..b9f4a09c744d 100644 --- a/drivers/media/pci/saa7146/hexium_orion.c +++ b/drivers/media/pci/saa7146/hexium_orion.c @@ -231,9 +231,7 @@ static int hexium_probe(struct saa7146_dev *dev) saa7146_write(dev, DD1_STREAM_B, 0x00000000); saa7146_write(dev, MC2, (MASK_09 | MASK_25 | MASK_10 | MASK_26)); - hexium->i2c_adapter = (struct i2c_adapter) { - .name = "hexium orion", - }; + strscpy(hexium->i2c_adapter.name, "hexium orion", sizeof(hexium->i2c_adapter.name)); saa7146_i2c_adapter_prepare(dev, &hexium->i2c_adapter, SAA7146_I2C_BUS_BIT_RATE_480); if (i2c_add_adapter(&hexium->i2c_adapter) < 0) { DEB_S("cannot register i2c-device. skipping.\n");
Two saa7146/hexium files contain a construct that causes a warning when built with clang: drivers/media/pci/saa7146/hexium_orion.c:210:12: error: stack frame size of 2272 bytes in function 'hexium_probe' [-Werror,-Wframe-larger-than=] static int hexium_probe(struct saa7146_dev *dev) ^ drivers/media/pci/saa7146/hexium_gemini.c:257:12: error: stack frame size of 2304 bytes in function 'hexium_attach' [-Werror,-Wframe-larger-than=] static int hexium_attach(struct saa7146_dev *dev, struct saa7146_pci_extension_data *info) ^ This one happens regardless of KASAN, and the problem is that a constructor to initalize a dynamically allocated structure leads to a copy of that structure on the stack, whereas gcc initializes it in place. Link: https://bugs.llvm.org/show_bug.cgi?id=40776 Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/media/pci/saa7146/hexium_gemini.c | 4 +--- drivers/media/pci/saa7146/hexium_orion.c | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) -- 2.20.0