All of lore.kernel.org
 help / color / mirror / Atom feed
* KASAN: global-out-of-bounds Write in string
@ 2018-04-03 17:01 syzbot
  2018-04-05 11:02 ` Tetsuo Handa
  2018-04-17  7:24 ` syzbot
  0 siblings, 2 replies; 5+ messages in thread
From: syzbot @ 2018-04-03 17:01 UTC (permalink / raw)
  To: dhowells, linux-kernel, reiserfs-devel, syzkaller-bugs

Hello,

syzbot hit the following crash on upstream commit
642e7fd23353e22290e3d51719fcb658dc252342 (Tue Apr 3 04:22:12 2018 +0000)
Merge branch 'syscalls-next' of  
git://git.kernel.org/pub/scm/linux/kernel/git/brodo/linux
syzbot dashboard link:  
https://syzkaller.appspot.com/bug?extid=b890b3335a4d8c608963

syzkaller reproducer:  
https://syzkaller.appspot.com/x/repro.syz?id=4698588919627776
Raw console output:  
https://syzkaller.appspot.com/x/log.txt?id=5456053378482176
Kernel config:  
https://syzkaller.appspot.com/x/.config?id=-6874493495260513980
compiler: gcc (GCC) 7.1.1 20170620

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+b890b3335a4d8c608963@syzkaller.appspotmail.com
It will help syzbot understand when the bug is fixed. See footer for  
details.
If you forward the report, please keep this part and the footer.

IPVS: ftp: loaded support on port[0] = 21
REISERFS warning (device loop0): sh-2021 reiserfs_fill_super: can not find  
reiserfs on loop0
syz-executor0 (4503) used greatest stack depth: 15560 bytes left
REISERFS warning (device loop0): sh-2021 reiserfs_fill_super: can not find  
reiserfs on loop0
==================================================================
BUG: KASAN: global-out-of-bounds in string+0x1cb/0x200 lib/vsprintf.c:598
Write of size 1 at addr ffffffff89e166a0 by task syz-executor0/4522

CPU: 1 PID: 4522 Comm: syz-executor0 Not tainted 4.16.0+ #12
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:17 [inline]
  dump_stack+0x1a7/0x27d lib/dump_stack.c:53
  print_address_description+0x178/0x250 mm/kasan/report.c:256
  kasan_report_error mm/kasan/report.c:354 [inline]
  kasan_report+0x23c/0x360 mm/kasan/report.c:412
  __asan_report_store1_noabort+0x17/0x20 mm/kasan/report.c:435
  string+0x1cb/0x200 lib/vsprintf.c:598
  vsnprintf+0x863/0x1900 lib/vsprintf.c:2282
  vsprintf+0x2a/0x40 lib/vsprintf.c:2462
  prepare_error_buf+0x1d2/0x1820 fs/reiserfs/prints.c:240
  __reiserfs_warning+0xc8/0x1a0 fs/reiserfs/prints.c:267
  reiserfs_getopt fs/reiserfs/super.c:1044 [inline]
  reiserfs_parse_options+0x11e5/0x24e0 fs/reiserfs/super.c:1194
  reiserfs_fill_super+0x520/0x33a0 fs/reiserfs/super.c:1946
  mount_bdev+0x2b7/0x370 fs/super.c:1119
  get_super_block+0x34/0x40 fs/reiserfs/super.c:2605
  mount_fs+0x66/0x2d0 fs/super.c:1222
  vfs_kern_mount.part.26+0xc6/0x4a0 fs/namespace.c:1037
  vfs_kern_mount fs/namespace.c:2514 [inline]
  do_new_mount fs/namespace.c:2517 [inline]
  do_mount+0xea4/0x2b90 fs/namespace.c:2847
  ksys_mount+0xab/0x120 fs/namespace.c:3063
  SYSC_mount fs/namespace.c:3077 [inline]
  SyS_mount+0x39/0x50 fs/namespace.c:3074
  do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x42/0xb7
RIP: 0033:0x457d0a
RSP: 002b:00007f0655cddbb8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0000000020000000 RCX: 0000000000457d0a
RDX: 0000000020000000 RSI: 0000000020000100 RDI: 00007f0655cddc00
RBP: 0000000000000004 R08: 0000000020013f00 R09: 0000000020000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000005
R13: 000000000000066d R14: 00000000006fcad8 R15: 0000000000000001

The buggy address belongs to the variable:
  error_buf+0x400/0x420

Memory state around the buggy address:
  ffffffff89e16580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffffffff89e16600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffffffff89e16680: 00 00 00 00 fa fa fa fa 04 fa fa fa fa fa fa fa
                                ^
  ffffffff89e16700: 00 fa fa fa fa fa fa fa 00 fa fa fa fa fa fa fa
  ffffffff89e16780: 00 fa fa fa fa fa fa fa 00 fa fa fa fa fa fa fa
==================================================================


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzkaller@googlegroups.com.

syzbot will keep track of this bug report.
If you forgot to add the Reported-by tag, once the fix for this bug is  
merged
into any tree, please reply to this email with:
#syz fix: exact-commit-title
If you want to test a patch for this bug, please reply with:
#syz test: git://repo/address.git branch
and provide the patch inline or as an attachment.
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug  
report.
Note: all commands must start from beginning of the line in the email body.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: KASAN: global-out-of-bounds Write in string
  2018-04-03 17:01 KASAN: global-out-of-bounds Write in string syzbot
@ 2018-04-05 11:02 ` Tetsuo Handa
  2018-04-17  7:24 ` syzbot
  1 sibling, 0 replies; 5+ messages in thread
From: Tetsuo Handa @ 2018-04-05 11:02 UTC (permalink / raw)
  To: dhowells, reiserfs-devel; +Cc: syzbot, linux-kernel, syzkaller-bugs

On 2018/04/04 2:01, syzbot wrote:
> BUG: KASAN: global-out-of-bounds in string+0x1cb/0x200 lib/vsprintf.c:598
> Write of size 1 at addr ffffffff89e166a0 by task syz-executor0/4522
> 
> CPU: 1 PID: 4522 Comm: syz-executor0 Not tainted 4.16.0+ #12
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:17 [inline]
>  dump_stack+0x1a7/0x27d lib/dump_stack.c:53
>  print_address_description+0x178/0x250 mm/kasan/report.c:256
>  kasan_report_error mm/kasan/report.c:354 [inline]
>  kasan_report+0x23c/0x360 mm/kasan/report.c:412
>  __asan_report_store1_noabort+0x17/0x20 mm/kasan/report.c:435
>  string+0x1cb/0x200 lib/vsprintf.c:598
>  vsnprintf+0x863/0x1900 lib/vsprintf.c:2282
>  vsprintf+0x2a/0x40 lib/vsprintf.c:2462
>  prepare_error_buf+0x1d2/0x1820 fs/reiserfs/prints.c:240
>  __reiserfs_warning+0xc8/0x1a0 fs/reiserfs/prints.c:267
>  reiserfs_getopt fs/reiserfs/super.c:1044 [inline]
>  reiserfs_parse_options+0x11e5/0x24e0 fs/reiserfs/super.c:1194
>  reiserfs_fill_super+0x520/0x33a0 fs/reiserfs/super.c:1946

> The buggy address belongs to the variable:
>  error_buf+0x400/0x420

I guess this is a buffer overflow bug due to

  static char error_buf[1024];
  char *p = error_buf;
  vsprintf(p, fmt1, args);

at prepare_error_buf(). Need to check available bytes.

> 
> Memory state around the buggy address:
>  ffffffff89e16580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  ffffffff89e16600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> ffffffff89e16680: 00 00 00 00 fa fa fa fa 04 fa fa fa fa fa fa fa
>                                ^
>  ffffffff89e16700: 00 fa fa fa fa fa fa fa 00 fa fa fa fa fa fa fa
>  ffffffff89e16780: 00 fa fa fa fa fa fa fa 00 fa fa fa fa fa fa fa
> ==================================================================

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: KASAN: global-out-of-bounds Write in string
  2018-04-03 17:01 KASAN: global-out-of-bounds Write in string syzbot
  2018-04-05 11:02 ` Tetsuo Handa
@ 2018-04-17  7:24 ` syzbot
  2018-07-07 20:36   ` [PATCH] reiserfs: fix buffer overflow with long warning messages Eric Biggers
  1 sibling, 1 reply; 5+ messages in thread
From: syzbot @ 2018-04-17  7:24 UTC (permalink / raw)
  To: dhowells, linux-kernel, penguin-kernel, reiserfs-devel, syzkaller-bugs

syzbot has found reproducer for the following crash on upstream commit
a27fc14219f2e3c4a46ba9177b04d9b52c875532 (Mon Apr 16 21:07:39 2018 +0000)
Merge branch 'parisc-4.17-3' of  
git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux
syzbot dashboard link:  
https://syzkaller.appspot.com/bug?extid=b890b3335a4d8c608963

So far this crash happened 3 times on upstream.
C reproducer: https://syzkaller.appspot.com/x/repro.c?id=4639432657338368
syzkaller reproducer:  
https://syzkaller.appspot.com/x/repro.syz?id=5739177354199040
Raw console output:  
https://syzkaller.appspot.com/x/log.txt?id=5837965024559104
Kernel config:  
https://syzkaller.appspot.com/x/.config?id=-5914490758943236750
compiler: gcc (GCC) 8.0.1 20180413 (experimental)

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+b890b3335a4d8c608963@syzkaller.appspotmail.com
It will help syzbot understand when the bug is fixed.

VFS: Can't find a Minix filesystem V1 | V2 | V3 on device loop5.
==================================================================
BUG: KASAN: global-out-of-bounds in string+0x291/0x2c0 lib/vsprintf.c:608
Write of size 1 at addr ffffffff8a5405c0 by task syzkaller915239/4487

CPU: 1 PID: 4487 Comm: syzkaller915239 Not tainted 4.17.0-rc1+ #6
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
  print_address_description+0x6c/0x20b mm/kasan/report.c:256
  kasan_report_error mm/kasan/report.c:354 [inline]
  kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
  __asan_report_store1_noabort+0x17/0x20 mm/kasan/report.c:435
  string+0x291/0x2c0 lib/vsprintf.c:608
  vsnprintf+0x4b7/0x1b40 lib/vsprintf.c:2292
  vsprintf+0x2a/0x40 lib/vsprintf.c:2472
  prepare_error_buf+0x23b/0x1aa0 fs/reiserfs/prints.c:240
  __reiserfs_warning+0xb0/0xd0 fs/reiserfs/prints.c:267
  reiserfs_getopt fs/reiserfs/super.c:1044 [inline]
  reiserfs_parse_options+0x11f4/0x27f0 fs/reiserfs/super.c:1194
  reiserfs_fill_super+0x520/0x3900 fs/reiserfs/super.c:1946
  mount_bdev+0x30c/0x3e0 fs/super.c:1165
  get_super_block+0x34/0x40 fs/reiserfs/super.c:2605
  mount_fs+0xae/0x328 fs/super.c:1268
  vfs_kern_mount.part.34+0xd4/0x4d0 fs/namespace.c:1037
  vfs_kern_mount fs/namespace.c:1027 [inline]
  do_new_mount fs/namespace.c:2517 [inline]
  do_mount+0x564/0x3070 fs/namespace.c:2847
  ksys_mount+0x12d/0x140 fs/namespace.c:3063
  __do_sys_mount fs/namespace.c:3077 [inline]
  __se_sys_mount fs/namespace.c:3074 [inline]
  __x64_sys_mount+0xbe/0x150 fs/namespace.c:3074
  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x448bca
RSP: 002b:00007f33571ccc68 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0000000020000040 RCX: 0000000000448bca
RDX: 0000000020000000 RSI: 0000000020000040 RDI: 00007f33571ccc80
RBP: 0000000000000004 R08: 0000000020000380 R09: 000000000000000a
R10: 0000000001000000 R11: 0000000000000202 R12: 0000000000000043
R13: 0000000000000044 R14: 0000000001000000 R15: 0000000000000004

The buggy address belongs to the variable:
  error_buf+0x400/0x420

Memory state around the buggy address:
  ffffffff8a540480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffffffff8a540500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffffffff8a540580: 00 00 00 00 00 00 00 00 fa fa fa fa 04 fa fa fa
                                            ^
  ffffffff8a540600: fa fa fa fa 00 fa fa fa fa fa fa fa 00 fa fa fa
  ffffffff8a540680: fa fa fa fa 00 fa fa fa fa fa fa fa 00 fa fa fa
==================================================================

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] reiserfs: fix buffer overflow with long warning messages
  2018-04-17  7:24 ` syzbot
@ 2018-07-07 20:36   ` Eric Biggers
  2018-07-16 15:25     ` Jan Kara
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Biggers @ 2018-07-07 20:36 UTC (permalink / raw)
  To: reiserfs-devel, Andrew Morton
  Cc: linux-kernel, syzkaller-bugs, Tetsuo Handa, Jeff Mahoney,
	Jan Kara, Alexander Viro, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

ReiserFS prepares log messages into a 1024-byte buffer with no bounds
checks.  Long messages, such as the "unknown mount option" warning when
userspace passes a crafted mount options string, overflow this buffer.
This causes KASAN to report a global-out-of-bounds write.

Fix it by truncating messages to the buffer size.

Reported-by: syzbot+b890b3335a4d8c608963@syzkaller.appspotmail.com
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/reiserfs/prints.c | 141 +++++++++++++++++++++++++------------------
 1 file changed, 81 insertions(+), 60 deletions(-)

diff --git a/fs/reiserfs/prints.c b/fs/reiserfs/prints.c
index 7e288d97adcbb..9fed1c05f1f4d 100644
--- a/fs/reiserfs/prints.c
+++ b/fs/reiserfs/prints.c
@@ -76,83 +76,99 @@ static char *le_type(struct reiserfs_key *key)
 }
 
 /* %k */
-static void sprintf_le_key(char *buf, struct reiserfs_key *key)
+static int scnprintf_le_key(char *buf, size_t size, struct reiserfs_key *key)
 {
 	if (key)
-		sprintf(buf, "[%d %d %s %s]", le32_to_cpu(key->k_dir_id),
-			le32_to_cpu(key->k_objectid), le_offset(key),
-			le_type(key));
+		return scnprintf(buf, size, "[%d %d %s %s]",
+				 le32_to_cpu(key->k_dir_id),
+				 le32_to_cpu(key->k_objectid), le_offset(key),
+				 le_type(key));
 	else
-		sprintf(buf, "[NULL]");
+		return scnprintf(buf, size, "[NULL]");
 }
 
 /* %K */
-static void sprintf_cpu_key(char *buf, struct cpu_key *key)
+static int scnprintf_cpu_key(char *buf, size_t size, struct cpu_key *key)
 {
 	if (key)
-		sprintf(buf, "[%d %d %s %s]", key->on_disk_key.k_dir_id,
-			key->on_disk_key.k_objectid, reiserfs_cpu_offset(key),
-			cpu_type(key));
+		return scnprintf(buf, size, "[%d %d %s %s]",
+				 key->on_disk_key.k_dir_id,
+				 key->on_disk_key.k_objectid,
+				 reiserfs_cpu_offset(key), cpu_type(key));
 	else
-		sprintf(buf, "[NULL]");
+		return scnprintf(buf, size, "[NULL]");
 }
 
-static void sprintf_de_head(char *buf, struct reiserfs_de_head *deh)
+static int scnprintf_de_head(char *buf, size_t size,
+			     struct reiserfs_de_head *deh)
 {
 	if (deh)
-		sprintf(buf,
-			"[offset=%d dir_id=%d objectid=%d location=%d state=%04x]",
-			deh_offset(deh), deh_dir_id(deh), deh_objectid(deh),
-			deh_location(deh), deh_state(deh));
+		return scnprintf(buf, size,
+				 "[offset=%d dir_id=%d objectid=%d location=%d state=%04x]",
+				 deh_offset(deh), deh_dir_id(deh),
+				 deh_objectid(deh), deh_location(deh),
+				 deh_state(deh));
 	else
-		sprintf(buf, "[NULL]");
+		return scnprintf(buf, size, "[NULL]");
 
 }
 
-static void sprintf_item_head(char *buf, struct item_head *ih)
+static int scnprintf_item_head(char *buf, size_t size, struct item_head *ih)
 {
 	if (ih) {
-		strcpy(buf,
-		       (ih_version(ih) == KEY_FORMAT_3_6) ? "*3.6* " : "*3.5*");
-		sprintf_le_key(buf + strlen(buf), &(ih->ih_key));
-		sprintf(buf + strlen(buf), ", item_len %d, item_location %d, "
-			"free_space(entry_count) %d",
-			ih_item_len(ih), ih_location(ih), ih_free_space(ih));
+		char *p = buf;
+		char * const end = buf + size;
+
+		p += scnprintf(p, end - p, "%s",
+			       (ih_version(ih) == KEY_FORMAT_3_6) ?
+			       "*3.6* " : "*3.5*");
+
+		p += scnprintf_le_key(p, end - p, &ih->ih_key);
+
+		p += scnprintf(p, end - p,
+			       ", item_len %d, item_location %d, free_space(entry_count) %d",
+			       ih_item_len(ih), ih_location(ih),
+			       ih_free_space(ih));
+		return p - buf;
 	} else
-		sprintf(buf, "[NULL]");
+		return scnprintf(buf, size, "[NULL]");
 }
 
-static void sprintf_direntry(char *buf, struct reiserfs_dir_entry *de)
+static int scnprintf_direntry(char *buf, size_t size,
+			      struct reiserfs_dir_entry *de)
 {
 	char name[20];
 
 	memcpy(name, de->de_name, de->de_namelen > 19 ? 19 : de->de_namelen);
 	name[de->de_namelen > 19 ? 19 : de->de_namelen] = 0;
-	sprintf(buf, "\"%s\"==>[%d %d]", name, de->de_dir_id, de->de_objectid);
+	return scnprintf(buf, size, "\"%s\"==>[%d %d]",
+			 name, de->de_dir_id, de->de_objectid);
 }
 
-static void sprintf_block_head(char *buf, struct buffer_head *bh)
+static int scnprintf_block_head(char *buf, size_t size, struct buffer_head *bh)
 {
-	sprintf(buf, "level=%d, nr_items=%d, free_space=%d rdkey ",
-		B_LEVEL(bh), B_NR_ITEMS(bh), B_FREE_SPACE(bh));
+	return scnprintf(buf, size,
+			 "level=%d, nr_items=%d, free_space=%d rdkey ",
+			 B_LEVEL(bh), B_NR_ITEMS(bh), B_FREE_SPACE(bh));
 }
 
-static void sprintf_buffer_head(char *buf, struct buffer_head *bh)
+static int scnprintf_buffer_head(char *buf, size_t size, struct buffer_head *bh)
 {
-	sprintf(buf,
-		"dev %pg, size %zd, blocknr %llu, count %d, state 0x%lx, page %p, (%s, %s, %s)",
-		bh->b_bdev, bh->b_size,
-		(unsigned long long)bh->b_blocknr, atomic_read(&(bh->b_count)),
-		bh->b_state, bh->b_page,
-		buffer_uptodate(bh) ? "UPTODATE" : "!UPTODATE",
-		buffer_dirty(bh) ? "DIRTY" : "CLEAN",
-		buffer_locked(bh) ? "LOCKED" : "UNLOCKED");
+	return scnprintf(buf, size,
+			 "dev %pg, size %zd, blocknr %llu, count %d, state 0x%lx, page %p, (%s, %s, %s)",
+			 bh->b_bdev, bh->b_size,
+			 (unsigned long long)bh->b_blocknr,
+			 atomic_read(&(bh->b_count)),
+			 bh->b_state, bh->b_page,
+			 buffer_uptodate(bh) ? "UPTODATE" : "!UPTODATE",
+			 buffer_dirty(bh) ? "DIRTY" : "CLEAN",
+			 buffer_locked(bh) ? "LOCKED" : "UNLOCKED");
 }
 
-static void sprintf_disk_child(char *buf, struct disk_child *dc)
+static int scnprintf_disk_child(char *buf, size_t size, struct disk_child *dc)
 {
-	sprintf(buf, "[dc_number=%d, dc_size=%u]", dc_block_number(dc),
-		dc_size(dc));
+	return scnprintf(buf, size, "[dc_number=%d, dc_size=%u]",
+			 dc_block_number(dc), dc_size(dc));
 }
 
 static char *is_there_reiserfs_struct(char *fmt, int *what)
@@ -189,55 +205,60 @@ static void prepare_error_buf(const char *fmt, va_list args)
 	char *fmt1 = fmt_buf;
 	char *k;
 	char *p = error_buf;
+	char * const end = &error_buf[sizeof(error_buf)];
 	int what;
 
 	spin_lock(&error_lock);
 
-	strcpy(fmt1, fmt);
+	if (WARN_ON(strscpy(fmt_buf, fmt, sizeof(fmt_buf)) < 0)) {
+		strscpy(error_buf, "format string too long", end - error_buf);
+		goto out_unlock;
+	}
 
 	while ((k = is_there_reiserfs_struct(fmt1, &what)) != NULL) {
 		*k = 0;
 
-		p += vsprintf(p, fmt1, args);
+		p += vscnprintf(p, end - p, fmt1, args);
 
 		switch (what) {
 		case 'k':
-			sprintf_le_key(p, va_arg(args, struct reiserfs_key *));
+			p += scnprintf_le_key(p, end - p,
+					      va_arg(args, struct reiserfs_key *));
 			break;
 		case 'K':
-			sprintf_cpu_key(p, va_arg(args, struct cpu_key *));
+			p += scnprintf_cpu_key(p, end - p,
+					       va_arg(args, struct cpu_key *));
 			break;
 		case 'h':
-			sprintf_item_head(p, va_arg(args, struct item_head *));
+			p += scnprintf_item_head(p, end - p,
+						 va_arg(args, struct item_head *));
 			break;
 		case 't':
-			sprintf_direntry(p,
-					 va_arg(args,
-						struct reiserfs_dir_entry *));
+			p += scnprintf_direntry(p, end - p,
+						va_arg(args, struct reiserfs_dir_entry *));
 			break;
 		case 'y':
-			sprintf_disk_child(p,
-					   va_arg(args, struct disk_child *));
+			p += scnprintf_disk_child(p, end - p,
+						  va_arg(args, struct disk_child *));
 			break;
 		case 'z':
-			sprintf_block_head(p,
-					   va_arg(args, struct buffer_head *));
+			p += scnprintf_block_head(p, end - p,
+						  va_arg(args, struct buffer_head *));
 			break;
 		case 'b':
-			sprintf_buffer_head(p,
-					    va_arg(args, struct buffer_head *));
+			p += scnprintf_buffer_head(p, end - p,
+						   va_arg(args, struct buffer_head *));
 			break;
 		case 'a':
-			sprintf_de_head(p,
-					va_arg(args,
-					       struct reiserfs_de_head *));
+			p += scnprintf_de_head(p, end - p,
+					       va_arg(args, struct reiserfs_de_head *));
 			break;
 		}
 
-		p += strlen(p);
 		fmt1 = k + 2;
 	}
-	vsprintf(p, fmt1, args);
+	p += vscnprintf(p, end - p, fmt1, args);
+out_unlock:
 	spin_unlock(&error_lock);
 
 }
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] reiserfs: fix buffer overflow with long warning messages
  2018-07-07 20:36   ` [PATCH] reiserfs: fix buffer overflow with long warning messages Eric Biggers
@ 2018-07-16 15:25     ` Jan Kara
  0 siblings, 0 replies; 5+ messages in thread
From: Jan Kara @ 2018-07-16 15:25 UTC (permalink / raw)
  To: Eric Biggers
  Cc: reiserfs-devel, Andrew Morton, linux-kernel, syzkaller-bugs,
	Tetsuo Handa, Jeff Mahoney, Jan Kara, Alexander Viro,
	Eric Biggers

On Sat 07-07-18 13:36:21, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> ReiserFS prepares log messages into a 1024-byte buffer with no bounds
> checks.  Long messages, such as the "unknown mount option" warning when
> userspace passes a crafted mount options string, overflow this buffer.
> This causes KASAN to report a global-out-of-bounds write.
> 
> Fix it by truncating messages to the buffer size.
> 
> Reported-by: syzbot+b890b3335a4d8c608963@syzkaller.appspotmail.com
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

I'd just note that I'd prefer some lines not to exceed 80 chars like:

-			sprintf_le_key(p, va_arg(args, struct reiserfs_key *));
+			p += scnprintf_le_key(p, end - p,
+					      va_arg(args, struct reiserfs_key *));

could be:

+			p += scnprintf_le_key(p, end - p,
+					va_arg(args, struct reiserfs_key *));

But I don't mind that much.

Andrew usually merges reiserfs patches. Andrew?

								Honza
> ---
>  fs/reiserfs/prints.c | 141 +++++++++++++++++++++++++------------------
>  1 file changed, 81 insertions(+), 60 deletions(-)
> 
> diff --git a/fs/reiserfs/prints.c b/fs/reiserfs/prints.c
> index 7e288d97adcbb..9fed1c05f1f4d 100644
> --- a/fs/reiserfs/prints.c
> +++ b/fs/reiserfs/prints.c
> @@ -76,83 +76,99 @@ static char *le_type(struct reiserfs_key *key)
>  }
>  
>  /* %k */
> -static void sprintf_le_key(char *buf, struct reiserfs_key *key)
> +static int scnprintf_le_key(char *buf, size_t size, struct reiserfs_key *key)
>  {
>  	if (key)
> -		sprintf(buf, "[%d %d %s %s]", le32_to_cpu(key->k_dir_id),
> -			le32_to_cpu(key->k_objectid), le_offset(key),
> -			le_type(key));
> +		return scnprintf(buf, size, "[%d %d %s %s]",
> +				 le32_to_cpu(key->k_dir_id),
> +				 le32_to_cpu(key->k_objectid), le_offset(key),
> +				 le_type(key));
>  	else
> -		sprintf(buf, "[NULL]");
> +		return scnprintf(buf, size, "[NULL]");
>  }
>  
>  /* %K */
> -static void sprintf_cpu_key(char *buf, struct cpu_key *key)
> +static int scnprintf_cpu_key(char *buf, size_t size, struct cpu_key *key)
>  {
>  	if (key)
> -		sprintf(buf, "[%d %d %s %s]", key->on_disk_key.k_dir_id,
> -			key->on_disk_key.k_objectid, reiserfs_cpu_offset(key),
> -			cpu_type(key));
> +		return scnprintf(buf, size, "[%d %d %s %s]",
> +				 key->on_disk_key.k_dir_id,
> +				 key->on_disk_key.k_objectid,
> +				 reiserfs_cpu_offset(key), cpu_type(key));
>  	else
> -		sprintf(buf, "[NULL]");
> +		return scnprintf(buf, size, "[NULL]");
>  }
>  
> -static void sprintf_de_head(char *buf, struct reiserfs_de_head *deh)
> +static int scnprintf_de_head(char *buf, size_t size,
> +			     struct reiserfs_de_head *deh)
>  {
>  	if (deh)
> -		sprintf(buf,
> -			"[offset=%d dir_id=%d objectid=%d location=%d state=%04x]",
> -			deh_offset(deh), deh_dir_id(deh), deh_objectid(deh),
> -			deh_location(deh), deh_state(deh));
> +		return scnprintf(buf, size,
> +				 "[offset=%d dir_id=%d objectid=%d location=%d state=%04x]",
> +				 deh_offset(deh), deh_dir_id(deh),
> +				 deh_objectid(deh), deh_location(deh),
> +				 deh_state(deh));
>  	else
> -		sprintf(buf, "[NULL]");
> +		return scnprintf(buf, size, "[NULL]");
>  
>  }
>  
> -static void sprintf_item_head(char *buf, struct item_head *ih)
> +static int scnprintf_item_head(char *buf, size_t size, struct item_head *ih)
>  {
>  	if (ih) {
> -		strcpy(buf,
> -		       (ih_version(ih) == KEY_FORMAT_3_6) ? "*3.6* " : "*3.5*");
> -		sprintf_le_key(buf + strlen(buf), &(ih->ih_key));
> -		sprintf(buf + strlen(buf), ", item_len %d, item_location %d, "
> -			"free_space(entry_count) %d",
> -			ih_item_len(ih), ih_location(ih), ih_free_space(ih));
> +		char *p = buf;
> +		char * const end = buf + size;
> +
> +		p += scnprintf(p, end - p, "%s",
> +			       (ih_version(ih) == KEY_FORMAT_3_6) ?
> +			       "*3.6* " : "*3.5*");
> +
> +		p += scnprintf_le_key(p, end - p, &ih->ih_key);
> +
> +		p += scnprintf(p, end - p,
> +			       ", item_len %d, item_location %d, free_space(entry_count) %d",
> +			       ih_item_len(ih), ih_location(ih),
> +			       ih_free_space(ih));
> +		return p - buf;
>  	} else
> -		sprintf(buf, "[NULL]");
> +		return scnprintf(buf, size, "[NULL]");
>  }
>  
> -static void sprintf_direntry(char *buf, struct reiserfs_dir_entry *de)
> +static int scnprintf_direntry(char *buf, size_t size,
> +			      struct reiserfs_dir_entry *de)
>  {
>  	char name[20];
>  
>  	memcpy(name, de->de_name, de->de_namelen > 19 ? 19 : de->de_namelen);
>  	name[de->de_namelen > 19 ? 19 : de->de_namelen] = 0;
> -	sprintf(buf, "\"%s\"==>[%d %d]", name, de->de_dir_id, de->de_objectid);
> +	return scnprintf(buf, size, "\"%s\"==>[%d %d]",
> +			 name, de->de_dir_id, de->de_objectid);
>  }
>  
> -static void sprintf_block_head(char *buf, struct buffer_head *bh)
> +static int scnprintf_block_head(char *buf, size_t size, struct buffer_head *bh)
>  {
> -	sprintf(buf, "level=%d, nr_items=%d, free_space=%d rdkey ",
> -		B_LEVEL(bh), B_NR_ITEMS(bh), B_FREE_SPACE(bh));
> +	return scnprintf(buf, size,
> +			 "level=%d, nr_items=%d, free_space=%d rdkey ",
> +			 B_LEVEL(bh), B_NR_ITEMS(bh), B_FREE_SPACE(bh));
>  }
>  
> -static void sprintf_buffer_head(char *buf, struct buffer_head *bh)
> +static int scnprintf_buffer_head(char *buf, size_t size, struct buffer_head *bh)
>  {
> -	sprintf(buf,
> -		"dev %pg, size %zd, blocknr %llu, count %d, state 0x%lx, page %p, (%s, %s, %s)",
> -		bh->b_bdev, bh->b_size,
> -		(unsigned long long)bh->b_blocknr, atomic_read(&(bh->b_count)),
> -		bh->b_state, bh->b_page,
> -		buffer_uptodate(bh) ? "UPTODATE" : "!UPTODATE",
> -		buffer_dirty(bh) ? "DIRTY" : "CLEAN",
> -		buffer_locked(bh) ? "LOCKED" : "UNLOCKED");
> +	return scnprintf(buf, size,
> +			 "dev %pg, size %zd, blocknr %llu, count %d, state 0x%lx, page %p, (%s, %s, %s)",
> +			 bh->b_bdev, bh->b_size,
> +			 (unsigned long long)bh->b_blocknr,
> +			 atomic_read(&(bh->b_count)),
> +			 bh->b_state, bh->b_page,
> +			 buffer_uptodate(bh) ? "UPTODATE" : "!UPTODATE",
> +			 buffer_dirty(bh) ? "DIRTY" : "CLEAN",
> +			 buffer_locked(bh) ? "LOCKED" : "UNLOCKED");
>  }
>  
> -static void sprintf_disk_child(char *buf, struct disk_child *dc)
> +static int scnprintf_disk_child(char *buf, size_t size, struct disk_child *dc)
>  {
> -	sprintf(buf, "[dc_number=%d, dc_size=%u]", dc_block_number(dc),
> -		dc_size(dc));
> +	return scnprintf(buf, size, "[dc_number=%d, dc_size=%u]",
> +			 dc_block_number(dc), dc_size(dc));
>  }
>  
>  static char *is_there_reiserfs_struct(char *fmt, int *what)
> @@ -189,55 +205,60 @@ static void prepare_error_buf(const char *fmt, va_list args)
>  	char *fmt1 = fmt_buf;
>  	char *k;
>  	char *p = error_buf;
> +	char * const end = &error_buf[sizeof(error_buf)];
>  	int what;
>  
>  	spin_lock(&error_lock);
>  
> -	strcpy(fmt1, fmt);
> +	if (WARN_ON(strscpy(fmt_buf, fmt, sizeof(fmt_buf)) < 0)) {
> +		strscpy(error_buf, "format string too long", end - error_buf);
> +		goto out_unlock;
> +	}
>  
>  	while ((k = is_there_reiserfs_struct(fmt1, &what)) != NULL) {
>  		*k = 0;
>  
> -		p += vsprintf(p, fmt1, args);
> +		p += vscnprintf(p, end - p, fmt1, args);
>  
>  		switch (what) {
>  		case 'k':
> -			sprintf_le_key(p, va_arg(args, struct reiserfs_key *));
> +			p += scnprintf_le_key(p, end - p,
> +					      va_arg(args, struct reiserfs_key *));
>  			break;
>  		case 'K':
> -			sprintf_cpu_key(p, va_arg(args, struct cpu_key *));
> +			p += scnprintf_cpu_key(p, end - p,
> +					       va_arg(args, struct cpu_key *));
>  			break;
>  		case 'h':
> -			sprintf_item_head(p, va_arg(args, struct item_head *));
> +			p += scnprintf_item_head(p, end - p,
> +						 va_arg(args, struct item_head *));
>  			break;
>  		case 't':
> -			sprintf_direntry(p,
> -					 va_arg(args,
> -						struct reiserfs_dir_entry *));
> +			p += scnprintf_direntry(p, end - p,
> +						va_arg(args, struct reiserfs_dir_entry *));
>  			break;
>  		case 'y':
> -			sprintf_disk_child(p,
> -					   va_arg(args, struct disk_child *));
> +			p += scnprintf_disk_child(p, end - p,
> +						  va_arg(args, struct disk_child *));
>  			break;
>  		case 'z':
> -			sprintf_block_head(p,
> -					   va_arg(args, struct buffer_head *));
> +			p += scnprintf_block_head(p, end - p,
> +						  va_arg(args, struct buffer_head *));
>  			break;
>  		case 'b':
> -			sprintf_buffer_head(p,
> -					    va_arg(args, struct buffer_head *));
> +			p += scnprintf_buffer_head(p, end - p,
> +						   va_arg(args, struct buffer_head *));
>  			break;
>  		case 'a':
> -			sprintf_de_head(p,
> -					va_arg(args,
> -					       struct reiserfs_de_head *));
> +			p += scnprintf_de_head(p, end - p,
> +					       va_arg(args, struct reiserfs_de_head *));
>  			break;
>  		}
>  
> -		p += strlen(p);
>  		fmt1 = k + 2;
>  	}
> -	vsprintf(p, fmt1, args);
> +	p += vscnprintf(p, end - p, fmt1, args);
> +out_unlock:
>  	spin_unlock(&error_lock);
>  
>  }
> -- 
> 2.18.0
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-07-16 15:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-03 17:01 KASAN: global-out-of-bounds Write in string syzbot
2018-04-05 11:02 ` Tetsuo Handa
2018-04-17  7:24 ` syzbot
2018-07-07 20:36   ` [PATCH] reiserfs: fix buffer overflow with long warning messages Eric Biggers
2018-07-16 15:25     ` Jan Kara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.