Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1737875 > unrolled thread
| Started by | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| First post | 2017-09-22 23:40 +0200 |
| Last post | 2017-09-26 08:50 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v4 4/9] em28xx: fix em28xx_dvb_init for KASAN Arnd Bergmann <arnd@arndb.de> - 2017-09-22 23:40 +0200
RE: [PATCH v4 4/9] em28xx: fix em28xx_dvb_init for KASAN David Laight <David.Laight@ACULAB.COM> - 2017-09-25 16:50 +0200
Re: [PATCH v4 4/9] em28xx: fix em28xx_dvb_init for KASAN Arnd Bergmann <arnd@arndb.de> - 2017-09-26 08:40 +0200
Re: [PATCH v4 4/9] em28xx: fix em28xx_dvb_init for KASAN Arnd Bergmann <arnd@arndb.de> - 2017-09-26 08:50 +0200
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-09-22 23:40 +0200 |
| Subject | [PATCH v4 4/9] em28xx: fix em28xx_dvb_init for KASAN |
| Message-ID | <usDPY-2NY-33@gated-at.bofh.it> |
With CONFIG_KASAN, the init function uses a large amount of kernel stack:
drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init.part.4':
drivers/media/usb/em28xx/em28xx-dvb.c:2061:1: error: the frame size of 3232 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
It seems that this is triggered in part by using strlcpy(), which the
compiler doesn't recognize as copying at most 'len' bytes, since strlcpy
is not part of the C standard.
It does however recognize the standard strncpy() and optimizes away
the extra checks for that, using only 1688 bytes in the end.
I have another larger patch that we could use in addition to this one,
in order to shrink the stack for -fsanitize-address-use-after-scope
(with gcc-7.1.1) as well, but that would not be appropriate for
stable backports, so let's focus on this one first.
Cc: stable@vger.kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/usb/em28xx/em28xx-dvb.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index 4a7db623fe29..06c363dc55ed 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -1440,7 +1440,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
tda10071_pdata.pll_multiplier = 20,
tda10071_pdata.tuner_i2c_addr = 0x14,
memset(&board_info, 0, sizeof(board_info));
- strlcpy(board_info.type, "tda10071_cx24118", I2C_NAME_SIZE);
+ strncpy(board_info.type, "tda10071_cx24118", I2C_NAME_SIZE - 1);
board_info.addr = 0x55;
board_info.platform_data = &tda10071_pdata;
request_module("tda10071");
@@ -1460,7 +1460,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
/* attach SEC */
a8293_pdata.dvb_frontend = dvb->fe[0];
memset(&board_info, 0, sizeof(board_info));
- strlcpy(board_info.type, "a8293", I2C_NAME_SIZE);
+ strncpy(board_info.type, "a8293", I2C_NAME_SIZE - 1);
board_info.addr = 0x08;
board_info.platform_data = &a8293_pdata;
request_module("a8293");
@@ -1643,7 +1643,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
m88ds3103_pdata.ts_clk_pol = 1;
m88ds3103_pdata.agc = 0x99;
memset(&board_info, 0, sizeof(board_info));
- strlcpy(board_info.type, "m88ds3103", I2C_NAME_SIZE);
+ strncpy(board_info.type, "m88ds3103", I2C_NAME_SIZE - 1);
board_info.addr = 0x68;
board_info.platform_data = &m88ds3103_pdata;
request_module("m88ds3103");
@@ -1664,7 +1664,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
/* attach tuner */
ts2020_config.fe = dvb->fe[0];
memset(&board_info, 0, sizeof(board_info));
- strlcpy(board_info.type, "ts2022", I2C_NAME_SIZE);
+ strncpy(board_info.type, "ts2022", I2C_NAME_SIZE - 1);
board_info.addr = 0x60;
board_info.platform_data = &ts2020_config;
request_module("ts2020");
@@ -1690,7 +1690,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
/* attach SEC */
a8293_pdata.dvb_frontend = dvb->fe[0];
memset(&board_info, 0, sizeof(board_info));
- strlcpy(board_info.type, "a8293", I2C_NAME_SIZE);
+ strncpy(board_info.type, "a8293", I2C_NAME_SIZE - 1);
board_info.addr = 0x08;
board_info.platform_data = &a8293_pdata;
request_module("a8293");
@@ -1729,7 +1729,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
si2168_config.fe = &dvb->fe[0];
si2168_config.ts_mode = SI2168_TS_PARALLEL;
memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "si2168", I2C_NAME_SIZE);
+ strncpy(info.type, "si2168", I2C_NAME_SIZE - 1);
info.addr = 0x64;
info.platform_data = &si2168_config;
request_module(info.type);
@@ -1755,7 +1755,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
si2157_config.mdev = dev->media_dev;
#endif
memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "si2157", I2C_NAME_SIZE);
+ strncpy(info.type, "si2157", I2C_NAME_SIZE - 1);
info.addr = 0x60;
info.platform_data = &si2157_config;
request_module(info.type);
@@ -1793,7 +1793,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
si2168_config.fe = &dvb->fe[0];
si2168_config.ts_mode = SI2168_TS_PARALLEL;
memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "si2168", I2C_NAME_SIZE);
+ strncpy(info.type, "si2168", I2C_NAME_SIZE - 1);
info.addr = 0x64;
info.platform_data = &si2168_config;
request_module(info.type);
@@ -1819,7 +1819,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
si2157_config.mdev = dev->media_dev;
#endif
memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "si2146", I2C_NAME_SIZE);
+ strncpy(info.type, "si2146", I2C_NAME_SIZE - 1);
info.addr = 0x60;
info.platform_data = &si2157_config;
request_module("si2157");
@@ -1853,7 +1853,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
/* attach demod */
memset(&tc90522_config, 0, sizeof(tc90522_config));
memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "tc90522sat", I2C_NAME_SIZE);
+ strncpy(info.type, "tc90522sat", I2C_NAME_SIZE - 1);
info.addr = 0x15;
info.platform_data = &tc90522_config;
request_module("tc90522");
@@ -1875,7 +1875,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
qm1d1c0042_config.fe = tc90522_config.fe;
qm1d1c0042_config.lpf = 1;
memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "qm1d1c0042", I2C_NAME_SIZE);
+ strncpy(info.type, "qm1d1c0042", I2C_NAME_SIZE - 1);
info.addr = 0x61;
info.platform_data = &qm1d1c0042_config;
request_module(info.type);
@@ -1913,7 +1913,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
si2168_config.fe = &dvb->fe[0];
si2168_config.ts_mode = SI2168_TS_SERIAL;
memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "si2168", I2C_NAME_SIZE);
+ strncpy(info.type, "si2168", I2C_NAME_SIZE - 1);
info.addr = 0x64;
info.platform_data = &si2168_config;
request_module(info.type);
@@ -1939,7 +1939,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
si2157_config.mdev = dev->media_dev;
#endif
memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "si2157", I2C_NAME_SIZE);
+ strncpy(info.type, "si2157", I2C_NAME_SIZE - 1);
info.addr = 0x60;
info.platform_data = &si2157_config;
request_module(info.type);
@@ -1975,7 +1975,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
lgdt3306a_config = hauppauge_01595_lgdt3306a_config;
lgdt3306a_config.fe = &dvb->fe[0];
lgdt3306a_config.i2c_adapter = &adapter;
- strlcpy(info.type, "lgdt3306a", sizeof(info.type));
+ strncpy(info.type, "lgdt3306a", sizeof(info.type) - 1);
info.addr = 0x59;
info.platform_data = &lgdt3306a_config;
request_module(info.type);
@@ -2002,7 +2002,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
si2157_config.mdev = dev->media_dev;
#endif
memset(&info, 0, sizeof(struct i2c_board_info));
- strlcpy(info.type, "si2157", sizeof(info.type));
+ strncpy(info.type, "si2157", sizeof(info.type) - 1);
info.addr = 0x60;
info.platform_data = &si2157_config;
request_module(info.type);
--
2.9.0
[toc] | [next] | [standalone]
| From | David Laight <David.Laight@ACULAB.COM> |
|---|---|
| Date | 2017-09-25 16:50 +0200 |
| Message-ID | <utCRP-7FH-1@gated-at.bofh.it> |
| In reply to | #1737875 |
From: Arnd Bergmann > Sent: 22 September 2017 22:29 ... > It seems that this is triggered in part by using strlcpy(), which the > compiler doesn't recognize as copying at most 'len' bytes, since strlcpy > is not part of the C standard. Neither is strncpy(). It'll almost certainly be a marker in a header file somewhere, so it should be possibly to teach it about other functions. David
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-09-26 08:40 +0200 |
| Message-ID | <utRHc-10R-3@gated-at.bofh.it> |
| In reply to | #1739094 |
On Mon, Sep 25, 2017 at 7:41 AM, David Laight <David.Laight@aculab.com> wrote:
> From: Arnd Bergmann
>> Sent: 22 September 2017 22:29
> ...
>> It seems that this is triggered in part by using strlcpy(), which the
>> compiler doesn't recognize as copying at most 'len' bytes, since strlcpy
>> is not part of the C standard.
>
> Neither is strncpy().
>
> It'll almost certainly be a marker in a header file somewhere,
> so it should be possibly to teach it about other functions.
I'm currently travelling and haven't investigated in detail, but from
taking a closer look here, I found that the hardened 'strlcpy()'
in include/linux/string.h triggers it. There is also a hardened
(much shorted) 'strncpy()' that doesn't trigger it in the same file,
and having only the extern declaration of strncpy also doesn't.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-09-26 08:50 +0200 |
| Message-ID | <utRQS-150-11@gated-at.bofh.it> |
| In reply to | #1739540 |
On Mon, Sep 25, 2017 at 11:32 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Sep 25, 2017 at 7:41 AM, David Laight <David.Laight@aculab.com> wrote:
>> From: Arnd Bergmann
>>> Sent: 22 September 2017 22:29
>> ...
>>> It seems that this is triggered in part by using strlcpy(), which the
>>> compiler doesn't recognize as copying at most 'len' bytes, since strlcpy
>>> is not part of the C standard.
>>
>> Neither is strncpy().
>>
>> It'll almost certainly be a marker in a header file somewhere,
>> so it should be possibly to teach it about other functions.
>
> I'm currently travelling and haven't investigated in detail, but from
> taking a closer look here, I found that the hardened 'strlcpy()'
> in include/linux/string.h triggers it. There is also a hardened
> (much shorted) 'strncpy()' that doesn't trigger it in the same file,
> and having only the extern declaration of strncpy also doesn't.
And a little more experimenting leads to this simple patch that fixes
the problem:
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -254,7 +254,7 @@ __FORTIFY_INLINE size_t strlcpy(char *p, const
char *q, size_t size)
size_t q_size = __builtin_object_size(q, 0);
if (p_size == (size_t)-1 && q_size == (size_t)-1)
return __real_strlcpy(p, q, size);
- ret = strlen(q);
+ ret = __builtin_strlen(q);
if (size) {
size_t len = (ret >= size) ? size - 1 : ret;
if (__builtin_constant_p(len) && len >= p_size)
The problem is apparently that the fortified strlcpy calls the fortified strlen,
which in turn calls strnlen and that ends up calling the extern '__real_strnlen'
that gcc cannot reduce to a constant expression for a constant input.
Not sure if that change is the best fix, but it seems to address the problem in
this driver and probably leads to better code in other places as well.
Arnd
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web