| From foo@baz Sun May 27 16:52:54 CEST 2018 |
| From: Arnd Bergmann <arnd@arndb.de> |
| Date: Tue, 20 Feb 2018 14:09:11 +0100 |
| Subject: md: raid5: avoid string overflow warning |
| |
| From: Arnd Bergmann <arnd@arndb.de> |
| |
| [ Upstream commit 53b8d89ddbdbb0e4625a46d2cdbb6f106c52f801 ] |
| |
| gcc warns about a possible overflow of the kmem_cache string, when adding |
| four characters to a string of the same length: |
| |
| drivers/md/raid5.c: In function 'setup_conf': |
| drivers/md/raid5.c:2207:34: error: '-alt' directive writing 4 bytes into a region of size between 1 and 32 [-Werror=format-overflow=] |
| sprintf(conf->cache_name[1], "%s-alt", conf->cache_name[0]); |
| ^~~~ |
| drivers/md/raid5.c:2207:2: note: 'sprintf' output between 5 and 36 bytes into a destination of size 32 |
| sprintf(conf->cache_name[1], "%s-alt", conf->cache_name[0]); |
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| |
| If I'm counting correctly, we need 11 characters for the fixed part |
| of the string and 18 characters for a 64-bit pointer (when no gendisk |
| is used), so that leaves three characters for conf->level, which should |
| always be sufficient. |
| |
| This makes the code use snprintf() with the correct length, to |
| make the code more robust against changes, and to get the compiler |
| to shut up. |
| |
| In commit f4be6b43f1ac ("md/raid5: ensure we create a unique name for |
| kmem_cache when mddev has no gendisk") from 2010, Neil said that |
| the pointer could be removed "shortly" once devices without gendisk |
| are disallowed. I have no idea if that happened, but if it did, that |
| should probably be changed as well. |
| |
| Signed-off-by: Arnd Bergmann <arnd@arndb.de> |
| Signed-off-by: Shaohua Li <sh.li@alibaba-inc.com> |
| Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/md/raid5.c | 7 ++++--- |
| 1 file changed, 4 insertions(+), 3 deletions(-) |
| |
| --- a/drivers/md/raid5.c |
| +++ b/drivers/md/raid5.c |
| @@ -2197,15 +2197,16 @@ static int grow_one_stripe(struct r5conf |
| static int grow_stripes(struct r5conf *conf, int num) |
| { |
| struct kmem_cache *sc; |
| + size_t namelen = sizeof(conf->cache_name[0]); |
| int devs = max(conf->raid_disks, conf->previous_raid_disks); |
| |
| if (conf->mddev->gendisk) |
| - sprintf(conf->cache_name[0], |
| + snprintf(conf->cache_name[0], namelen, |
| "raid%d-%s", conf->level, mdname(conf->mddev)); |
| else |
| - sprintf(conf->cache_name[0], |
| + snprintf(conf->cache_name[0], namelen, |
| "raid%d-%p", conf->level, conf->mddev); |
| - sprintf(conf->cache_name[1], "%s-alt", conf->cache_name[0]); |
| + snprintf(conf->cache_name[1], namelen, "%.27s-alt", conf->cache_name[0]); |
| |
| conf->active_name = 0; |
| sc = kmem_cache_create(conf->cache_name[conf->active_name], |