* [PATCH 1/2] btrfs: Fix wild memory access in compression level parser
@ 2017-11-06 2:43 Qu Wenruo
2017-11-06 2:43 ` [PATCH 2/2] btrfs: compression: Return correct default zlib compression level Qu Wenruo
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Qu Wenruo @ 2017-11-06 2:43 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, lakshmipathi.g
[BUG]
Kernel panic when mounting with "-o compress" mount option.
KASAN will report like:
------
==================================================================
BUG: KASAN: wild-memory-access in strncmp+0x31/0xc0
Read of size 1 at addr d86735fce994f800 by task mount/662
...
Call Trace:
dump_stack+0xe3/0x175
kasan_report+0x163/0x370
__asan_load1+0x47/0x50
strncmp+0x31/0xc0
btrfs_compress_str2level+0x20/0x70 [btrfs]
btrfs_parse_options+0xff4/0x1870 [btrfs]
open_ctree+0x2679/0x49f0 [btrfs]
btrfs_mount+0x1b7f/0x1d30 [btrfs]
mount_fs+0x49/0x190
vfs_kern_mount.part.29+0xba/0x280
vfs_kern_mount+0x13/0x20
btrfs_mount+0x31e/0x1d30 [btrfs]
mount_fs+0x49/0x190
vfs_kern_mount.part.29+0xba/0x280
do_mount+0xaad/0x1a00
SyS_mount+0x98/0xe0
entry_SYSCALL_64_fastpath+0x1f/0xbe
------
[Cause]
For 'compress' and 'compress_force' options, its token doesn't expect
any parameter so its args[0] contains uninitialized data.
Accessing args[0] will cause above wild memory access.
[Fix]
For Opt_compress and Opt_compress_force, set compression level to
Z_DEFAULT_COMPRESSION manually.
NOTE: Don't set zlib compression level to 0 by default, which means no
compression.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/super.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 65af029559b5..14258671da84 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -42,6 +42,7 @@
#include <linux/cleancache.h>
#include <linux/ratelimit.h>
#include <linux/btrfs.h>
+#include <linux/zlib.h>
#include "delayed-inode.h"
#include "ctree.h"
#include "disk-io.h"
@@ -507,8 +508,19 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
token == Opt_compress_force ||
strncmp(args[0].from, "zlib", 4) == 0) {
compress_type = "zlib";
+
info->compress_type = BTRFS_COMPRESS_ZLIB;
- info->compress_level =
+ /*
+ * args[0] contains uninitialized data since
+ * for these tokens we don't expect any
+ * parameter.
+ */
+ if (token == Opt_compress ||
+ token == Opt_compress_force)
+ info->compress_level =
+ Z_DEFAULT_COMPRESSION;
+ else
+ info->compress_level =
btrfs_compress_str2level(args[0].from);
btrfs_set_opt(info->mount_opt, COMPRESS);
btrfs_clear_opt(info->mount_opt, NODATACOW);
--
2.14.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] btrfs: compression: Return correct default zlib compression level
2017-11-06 2:43 [PATCH 1/2] btrfs: Fix wild memory access in compression level parser Qu Wenruo
@ 2017-11-06 2:43 ` Qu Wenruo
2017-11-06 9:52 ` [PATCH 1/2] btrfs: Fix wild memory access in compression level parser Lu Fengqi
2017-11-15 15:11 ` David Sterba
2 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2017-11-06 2:43 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, lakshmipathi.g
0 means no compression at all for zlib.
Z_DEFAULT_COMPRESSION is the correct value, which is -1 and will be
converted to 6 at runtime.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/compression.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index b35ce16b3df3..29a41e6c9e28 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -35,6 +35,7 @@
#include <linux/sched/mm.h>
#include <linux/sort.h>
#include <linux/log2.h>
+#include <linux/zlib.h>
#include "ctree.h"
#include "disk-io.h"
#include "transaction.h"
@@ -1528,5 +1529,9 @@ unsigned int btrfs_compress_str2level(const char *str)
if (str[4] == ':' && '1' <= str[5] && str[5] <= '9' && str[6] == 0)
return str[5] - '0';
- return 0;
+ /*
+ * NOTE: Default compression level is not 0!!
+ * 0 means no compression at all
+ */
+ return Z_DEFAULT_COMPRESSION;
}
--
2.14.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] btrfs: Fix wild memory access in compression level parser
2017-11-06 2:43 [PATCH 1/2] btrfs: Fix wild memory access in compression level parser Qu Wenruo
2017-11-06 2:43 ` [PATCH 2/2] btrfs: compression: Return correct default zlib compression level Qu Wenruo
@ 2017-11-06 9:52 ` Lu Fengqi
2017-11-15 15:11 ` David Sterba
2 siblings, 0 replies; 6+ messages in thread
From: Lu Fengqi @ 2017-11-06 9:52 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, dsterba, lakshmipathi.g
On Mon, Nov 06, 2017 at 10:43:18AM +0800, Qu Wenruo wrote:
>[BUG]
>Kernel panic when mounting with "-o compress" mount option.
>KASAN will report like:
>------
>==================================================================
>BUG: KASAN: wild-memory-access in strncmp+0x31/0xc0
>Read of size 1 at addr d86735fce994f800 by task mount/662
>...
>Call Trace:
> dump_stack+0xe3/0x175
> kasan_report+0x163/0x370
> __asan_load1+0x47/0x50
> strncmp+0x31/0xc0
> btrfs_compress_str2level+0x20/0x70 [btrfs]
> btrfs_parse_options+0xff4/0x1870 [btrfs]
> open_ctree+0x2679/0x49f0 [btrfs]
> btrfs_mount+0x1b7f/0x1d30 [btrfs]
> mount_fs+0x49/0x190
> vfs_kern_mount.part.29+0xba/0x280
> vfs_kern_mount+0x13/0x20
> btrfs_mount+0x31e/0x1d30 [btrfs]
> mount_fs+0x49/0x190
> vfs_kern_mount.part.29+0xba/0x280
> do_mount+0xaad/0x1a00
> SyS_mount+0x98/0xe0
> entry_SYSCALL_64_fastpath+0x1f/0xbe
>------
>
>[Cause]
>For 'compress' and 'compress_force' options, its token doesn't expect
>any parameter so its args[0] contains uninitialized data.
>Accessing args[0] will cause above wild memory access.
>
>[Fix]
>For Opt_compress and Opt_compress_force, set compression level to
>Z_DEFAULT_COMPRESSION manually.
>
>NOTE: Don't set zlib compression level to 0 by default, which means no
>compression.
>
>Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
--
Thanks,
Lu
>---
> fs/btrfs/super.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
>diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>index 65af029559b5..14258671da84 100644
>--- a/fs/btrfs/super.c
>+++ b/fs/btrfs/super.c
>@@ -42,6 +42,7 @@
> #include <linux/cleancache.h>
> #include <linux/ratelimit.h>
> #include <linux/btrfs.h>
>+#include <linux/zlib.h>
> #include "delayed-inode.h"
> #include "ctree.h"
> #include "disk-io.h"
>@@ -507,8 +508,19 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
> token == Opt_compress_force ||
> strncmp(args[0].from, "zlib", 4) == 0) {
> compress_type = "zlib";
>+
> info->compress_type = BTRFS_COMPRESS_ZLIB;
>- info->compress_level =
>+ /*
>+ * args[0] contains uninitialized data since
>+ * for these tokens we don't expect any
>+ * parameter.
>+ */
>+ if (token == Opt_compress ||
>+ token == Opt_compress_force)
>+ info->compress_level =
>+ Z_DEFAULT_COMPRESSION;
>+ else
>+ info->compress_level =
> btrfs_compress_str2level(args[0].from);
> btrfs_set_opt(info->mount_opt, COMPRESS);
> btrfs_clear_opt(info->mount_opt, NODATACOW);
>--
>2.14.3
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] btrfs: Fix wild memory access in compression level parser
2017-11-06 2:43 [PATCH 1/2] btrfs: Fix wild memory access in compression level parser Qu Wenruo
2017-11-06 2:43 ` [PATCH 2/2] btrfs: compression: Return correct default zlib compression level Qu Wenruo
2017-11-06 9:52 ` [PATCH 1/2] btrfs: Fix wild memory access in compression level parser Lu Fengqi
@ 2017-11-15 15:11 ` David Sterba
2017-11-16 0:49 ` Qu Wenruo
2 siblings, 1 reply; 6+ messages in thread
From: David Sterba @ 2017-11-15 15:11 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, dsterba, lakshmipathi.g
On Mon, Nov 06, 2017 at 10:43:18AM +0800, Qu Wenruo wrote:
> [BUG]
> Kernel panic when mounting with "-o compress" mount option.
> KASAN will report like:
> ------
> ==================================================================
> BUG: KASAN: wild-memory-access in strncmp+0x31/0xc0
> Read of size 1 at addr d86735fce994f800 by task mount/662
> ...
> Call Trace:
> dump_stack+0xe3/0x175
> kasan_report+0x163/0x370
> __asan_load1+0x47/0x50
> strncmp+0x31/0xc0
> btrfs_compress_str2level+0x20/0x70 [btrfs]
> btrfs_parse_options+0xff4/0x1870 [btrfs]
> open_ctree+0x2679/0x49f0 [btrfs]
> btrfs_mount+0x1b7f/0x1d30 [btrfs]
> mount_fs+0x49/0x190
> vfs_kern_mount.part.29+0xba/0x280
> vfs_kern_mount+0x13/0x20
> btrfs_mount+0x31e/0x1d30 [btrfs]
> mount_fs+0x49/0x190
> vfs_kern_mount.part.29+0xba/0x280
> do_mount+0xaad/0x1a00
> SyS_mount+0x98/0xe0
> entry_SYSCALL_64_fastpath+0x1f/0xbe
> ------
>
> [Cause]
> For 'compress' and 'compress_force' options, its token doesn't expect
> any parameter so its args[0] contains uninitialized data.
> Accessing args[0] will cause above wild memory access.
>
> [Fix]
> For Opt_compress and Opt_compress_force, set compression level to
> Z_DEFAULT_COMPRESSION manually.
>
> NOTE: Don't set zlib compression level to 0 by default, which means no
> compression.
But we never set the level to 0 at the point the compression actually
happens. See zlib.c:zlib_set_level, if level is 0 then the level
passed to zlib is 3. Z_DEFAULT_COMPRESSION is upstream zlib level 6,
which is slower, we need zlib to stay in the real-time numbers.
> @@ -507,8 +508,19 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
> token == Opt_compress_force ||
> strncmp(args[0].from, "zlib", 4) == 0) {
> compress_type = "zlib";
> +
> info->compress_type = BTRFS_COMPRESS_ZLIB;
> - info->compress_level =
> + /*
> + * args[0] contains uninitialized data since
> + * for these tokens we don't expect any
> + * parameter.
> + */
> + if (token == Opt_compress ||
> + token == Opt_compress_force)
> + info->compress_level =
> + Z_DEFAULT_COMPRESSION;
> + else
> + info->compress_level =
> btrfs_compress_str2level(args[0].from);
At least this will not screw up the levels, anything that's not
recognized will become the default.
> btrfs_set_opt(info->mount_opt, COMPRESS);
> btrfs_clear_opt(info->mount_opt, NODATACOW);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] btrfs: Fix wild memory access in compression level parser
2017-11-15 15:11 ` David Sterba
@ 2017-11-16 0:49 ` Qu Wenruo
2017-11-21 15:25 ` David Sterba
0 siblings, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2017-11-16 0:49 UTC (permalink / raw)
To: dsterba, Qu Wenruo, linux-btrfs, lakshmipathi.g
[-- Attachment #1.1: Type: text/plain, Size: 2997 bytes --]
On 2017年11月15日 23:11, David Sterba wrote:
> On Mon, Nov 06, 2017 at 10:43:18AM +0800, Qu Wenruo wrote:
>> [BUG]
>> Kernel panic when mounting with "-o compress" mount option.
>> KASAN will report like:
>> ------
>> ==================================================================
>> BUG: KASAN: wild-memory-access in strncmp+0x31/0xc0
>> Read of size 1 at addr d86735fce994f800 by task mount/662
>> ...
>> Call Trace:
>> dump_stack+0xe3/0x175
>> kasan_report+0x163/0x370
>> __asan_load1+0x47/0x50
>> strncmp+0x31/0xc0
>> btrfs_compress_str2level+0x20/0x70 [btrfs]
>> btrfs_parse_options+0xff4/0x1870 [btrfs]
>> open_ctree+0x2679/0x49f0 [btrfs]
>> btrfs_mount+0x1b7f/0x1d30 [btrfs]
>> mount_fs+0x49/0x190
>> vfs_kern_mount.part.29+0xba/0x280
>> vfs_kern_mount+0x13/0x20
>> btrfs_mount+0x31e/0x1d30 [btrfs]
>> mount_fs+0x49/0x190
>> vfs_kern_mount.part.29+0xba/0x280
>> do_mount+0xaad/0x1a00
>> SyS_mount+0x98/0xe0
>> entry_SYSCALL_64_fastpath+0x1f/0xbe
>> ------
>>
>> [Cause]
>> For 'compress' and 'compress_force' options, its token doesn't expect
>> any parameter so its args[0] contains uninitialized data.
>> Accessing args[0] will cause above wild memory access.
>>
>> [Fix]
>> For Opt_compress and Opt_compress_force, set compression level to
>> Z_DEFAULT_COMPRESSION manually.
>>
>> NOTE: Don't set zlib compression level to 0 by default, which means no
>> compression.
>
> But we never set the level to 0 at the point the compression actually
> happens. See zlib.c:zlib_set_level, if level is 0 then the level
> passed to zlib is 3. Z_DEFAULT_COMPRESSION is upstream zlib level 6,
> which is slower, we need zlib to stay in the real-time numbers.
Right, I missed that.
So should I still use 0, or use separate macro like
BTRFS_DEFAULT_ZLIB_LEVEL?
Thanks,
Qu
>
>> @@ -507,8 +508,19 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
>> token == Opt_compress_force ||
>> strncmp(args[0].from, "zlib", 4) == 0) {
>> compress_type = "zlib";
>> +
>> info->compress_type = BTRFS_COMPRESS_ZLIB;
>> - info->compress_level =
>> + /*
>> + * args[0] contains uninitialized data since
>> + * for these tokens we don't expect any
>> + * parameter.
>> + */
>> + if (token == Opt_compress ||
>> + token == Opt_compress_force)
>> + info->compress_level =
>> + Z_DEFAULT_COMPRESSION;
>> + else
>> + info->compress_level =
>> btrfs_compress_str2level(args[0].from);
>
> At least this will not screw up the levels, anything that's not
> recognized will become the default.
>
>> btrfs_set_opt(info->mount_opt, COMPRESS);
>> btrfs_clear_opt(info->mount_opt, NODATACOW);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] btrfs: Fix wild memory access in compression level parser
2017-11-16 0:49 ` Qu Wenruo
@ 2017-11-21 15:25 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2017-11-21 15:25 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Qu Wenruo, linux-btrfs, lakshmipathi.g
On Thu, Nov 16, 2017 at 08:49:47AM +0800, Qu Wenruo wrote:
> > But we never set the level to 0 at the point the compression actually
> > happens. See zlib.c:zlib_set_level, if level is 0 then the level
> > passed to zlib is 3. Z_DEFAULT_COMPRESSION is upstream zlib level 6,
> > which is slower, we need zlib to stay in the real-time numbers.
>
> Right, I missed that.
>
> So should I still use 0, or use separate macro like
> BTRFS_DEFAULT_ZLIB_LEVEL?
BTRFS_DEFAULT_ZLIB_LEVEL would be better, as this would address
https://patchwork.kernel.org/patch/10021441/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-21 15:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-06 2:43 [PATCH 1/2] btrfs: Fix wild memory access in compression level parser Qu Wenruo
2017-11-06 2:43 ` [PATCH 2/2] btrfs: compression: Return correct default zlib compression level Qu Wenruo
2017-11-06 9:52 ` [PATCH 1/2] btrfs: Fix wild memory access in compression level parser Lu Fengqi
2017-11-15 15:11 ` David Sterba
2017-11-16 0:49 ` Qu Wenruo
2017-11-21 15:25 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).