Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

UBSAN: signed-integer-overflow in ../fs/read_write.c:91:10 #358

Open
JustinStitt opened this issue May 7, 2024 · 4 comments
Open

UBSAN: signed-integer-overflow in ../fs/read_write.c:91:10 #358

JustinStitt opened this issue May 7, 2024 · 4 comments
Assignees
@JustinStitt
Labels
[PATCH] Submitted A patch has been submitted upstream

Comments

@JustinStitt
Copy link
Collaborator 

[   67.991989] ------------[ cut here ]------------
[   67.995501] UBSAN: signed-integer-overflow in ../fs/read_write.c:91:10
[   68.000067] 9223372036854775807 + 4096 cannot be represented in type 'loff_t' (aka 'long long')
[   68.006266] CPU: 4 PID: 10851 Comm: syz-executor.5 Not tainted 6.8.0-rc2-00035-gb3ef86b5a957 #1
[   68.012353] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[   68.018983] Call Trace:
[   68.020803]  <TASK>
[   68.022540]  dump_stack_lvl+0x93/0xd0
[   68.025222]  handle_overflow+0x171/0x1b0
[   68.028053]  generic_file_llseek_size+0x35b/0x380
[   68.031298]  ? generic_file_llseek+0x25/0xd0
[   68.034350]  kernfs_fop_llseek+0x12c/0x170
[   68.037312]  __x64_sys_lseek+0x14d/0x1b0
[   68.040091]  do_syscall_64+0xd4/0x1b0
[   68.042751]  ? arch_exit_to_user_mode_prepare+0x11/0x60
[   68.046454]  entry_SYSCALL_64_after_hwframe+0x6f/0x77
[   68.046471] RIP: 0033:0x7f305def5539
[   68.046477] Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 14 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
[   68.046498] RSP: 002b:00007f305d28d0c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000008
[   68.046508] RAX: ffffffffffffffda RBX: 00007f305e029f80 RCX: 00007f305def5539
[   68.046514] RDX: 0000000000000002 RSI: 7fffffffffffffff RDI: 0000000000000003
[   68.046518] RBP: 00007f305df54496 R08: 0000000000000000 R09: 0000000000000000
[   68.046523] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[   68.046528] R13: 0000000000000002 R14: 00007f305e029f80 R15: 00007ffe726866f8
[   68.046543]  </TASK>
[   68.046746] ---[ end trace ]---
[   68.956760] 9pnet_fd: Insufficient options for proto=fd
[   68.979618] 9pnet_fd: Insufficient options for proto=fd
[   68.990464] 9pnet_fd: Insufficient options for proto=fd
@JustinStitt JustinStitt self-assigned this May 7, 2024
@JustinStitt
Copy link
Collaborator Author

JustinStitt commented May 9, 2024 •
edited
Loading

[PATCH] fs: fix unintentional arithmetic wraparound in offset calculation

@JustinStitt JustinStitt added the [PATCH] Submitted A patch has been submitted upstream label May 9, 2024
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue May 9, 2024
@intel-lab-lkp
fs: fix unintentional arithmetic wraparound in offset calculation
707eb91 
When running syzkaller with the newly reintroduced signed integer
overflow sanitizer we encounter this report:

[   67.991989] ------------[ cut here ]------------
[   67.995501] UBSAN: signed-integer-overflow in ../fs/read_write.c:91:10
[   68.000067] 9223372036854775807 + 4096 cannot be represented in type 'loff_t' (aka 'long long')
[   68.006266] CPU: 4 PID: 10851 Comm: syz-executor.5 Not tainted 6.8.0-rc2-00035-gb3ef86b5a957 #1
[   68.012353] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[   68.018983] Call Trace:
[   68.020803]  <TASK>
[   68.022540]  dump_stack_lvl+0x93/0xd0
[   68.025222]  handle_overflow+0x171/0x1b0
[   68.028053]  generic_file_llseek_size+0x35b/0x380
..

Historically, the signed integer overflow sanitizer did not work in the
kernel due to its interaction with `-fwrapv` but this has since been
changed [1] in the newest version of Clang. It was re-enabled in the
kernel with Commit 557f8c5 ("ubsan: Reintroduce signed overflow
sanitizer").

Since @offset is later limited by @maxsize, we can proactively safeguard
against exceeding that value and also dodge some accidental overflow
(which may cause bad file access):

	loff_t vfs_setpos(struct file *file, loff_t offset, loff_t maxsize)
	{
		if (offset < 0 && !unsigned_offsets(file))
			return -EINVAL;
		if (offset > maxsize)
			return -EINVAL;
		...

Link: llvm/llvm-project#82432 [1]
Closes: KSPP#358
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
@JustinStitt
Copy link
Collaborator Author

Looks like there's another instance in the same file:

[   50.998938] UBSAN: signed-integer-overflow in ../fs/read_write.c:108:41
[   51.009811] 4 + 9223372036854775807 cannot be represented in type 'loff_t' (aka 'long long')
[   51.009823] CPU: 0 PID: 7003 Comm: syz-executor.1 Not tainted 6.8.0-rc2-00035-gb3ef86b5a957 #1
[   51.009838] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[   51.033007] Call Trace:
[   51.035388]  <TASK>
[   51.037352]  dump_stack_lvl+0x93/0xd0
[   51.040482]  handle_overflow+0x171/0x1b0
[   51.044130]  generic_file_llseek_size+0x377/0x380
[   51.048246]  __x64_sys_lseek+0x14d/0x1b0
[   51.051659]  do_syscall_64+0xd4/0x1b0
[   51.054889]  ? arch_exit_to_user_mode_prepare+0x11/0x60
[   51.059595]  entry_SYSCALL_64_after_hwframe+0x6f/0x77
[   51.064031] RIP: 0033:0x7f3a5c75c539
[   51.067312] Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 14 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
[   51.080035] RSP: 002b:00007f3a5baf40c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000008
[   51.084867] RAX: ffffffffffffffda RBX: 00007f3a5c890f80 RCX: 00007f3a5c75c539
[   51.089767] RDX: 0000000000000001 RSI: 7fffffffffffffff RDI: 0000000000000003
[   51.094546] RBP: 00007f3a5c7bb496 R08: 0000000000000000 R09: 0000000000000000
[   51.099230] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[   51.104080] R13: 0000000000000002 R14: 00007f3a5c890f80 R15: 00007fffdd89eb28
[   51.109259]  </TASK>
[   51.112629] ---[ end trace ]---

I'll send a v2 shortly addressing this newly found instance.

@JustinStitt
Copy link
Collaborator Author

JustinStitt commented May 9, 2024 •
edited
Loading

And another!

UBSAN: signed-integer-overflow in ../fs/read_write.c:1666:11
9223372036854775807 - -9223231299366420479 cannot be represented in type 'loff_t' (aka 'long long')
CPU: 5 PID: 23543 Comm: syz-executor.2 Not tainted 6.8.0-rc2-00035-gb3ef86b5a957 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x93/0xd0 lib/dump_stack.c:106
 ubsan_epilogue lib/ubsan.c:217 [inline]
 handle_overflow+0x171/0x1b0 lib/ubsan.c:248
 generic_write_check_limits fs/read_write.c:1666 [inline]
 generic_copy_file_checks fs/read_write.c:1464 [inline]
...

... and here

UBSAN: signed-integer-overflow in ../fs/read_write.c:1657:12
142606336 - -9223372036854775807 cannot be represented in type 'loff_t' (aka 'long long')
CPU: 7 PID: 18813 Comm: syz-executor.1 Not tainted 6.8.0-rc2-00035-gb3ef86b5a957 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x93/0xd0 lib/dump_stack.c:106
 ubsan_epilogue lib/ubsan.c:217 [inline]
 handle_overflow+0x171/0x1b0 lib/ubsan.c:248
 generic_write_check_limits fs/read_write.c:1657 [inline]
 generic_copy_file_checks fs/read_write.c:1464 [inline]
...

@JustinStitt
Copy link
Collaborator Author

JustinStitt commented May 9, 2024 •
edited
Loading

v3: https://lore.kernel.org/all/20240517-b4-sio-read_write-v3-1-f180df0a19e6@google.com/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JustinStitt JustinStitt

Labels
[PATCH] Submitted A patch has been submitted upstream
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@JustinStitt