* [PATCH AUTOSEL 6.1 02/29] scsi: sr: Fix unintentional arithmetic wraparound
[not found] <20240617132456.2588952-1-sashal@kernel.org>
@ 2024-06-17 13:24 ` Sasha Levin
0 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2024-06-17 13:24 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Justin Stitt, linux-hardening, Kees Cook, Martin K . Petersen,
Sasha Levin, phil, corbet, James.Bottomley, nathan, linux-doc,
linux-scsi, llvm
From: Justin Stitt <justinstitt@google.com>
[ Upstream commit 9fad9d560af5c654bb38e0b07ee54a4e9acdc5cd ]
Running syzkaller with the newly reintroduced signed integer overflow
sanitizer produces this report:
[ 65.194362] ------------[ cut here ]------------
[ 65.197752] UBSAN: signed-integer-overflow in ../drivers/scsi/sr_ioctl.c:436:9
[ 65.203607] -2147483648 * 177 cannot be represented in type 'int'
[ 65.207911] CPU: 2 PID: 10416 Comm: syz-executor.1 Not tainted 6.8.0-rc2-00035-gb3ef86b5a957 #1
[ 65.213585] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 65.219923] Call Trace:
[ 65.221556] <TASK>
[ 65.223029] dump_stack_lvl+0x93/0xd0
[ 65.225573] handle_overflow+0x171/0x1b0
[ 65.228219] sr_select_speed+0xeb/0xf0
[ 65.230786] ? __pm_runtime_resume+0xe6/0x130
[ 65.233606] sr_block_ioctl+0x15d/0x1d0
...
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 557f8c582a9b ("ubsan: Reintroduce signed overflow sanitizer").
Firstly, let's change the type of "speed" to unsigned long as
sr_select_speed()'s only caller passes in an unsigned long anyways.
$ git grep '\.select_speed'
| drivers/scsi/sr.c: .select_speed = sr_select_speed,
...
| static int cdrom_ioctl_select_speed(struct cdrom_device_info *cdi,
| unsigned long arg)
| {
| ...
| return cdi->ops->select_speed(cdi, arg);
| }
Next, let's add an extra check to make sure we don't exceed 0xffff/177
(350) since 0xffff is the max speed. This has two benefits: 1) we deal
with integer overflow before it happens and 2) we properly respect the
max speed of 0xffff. There are some "magic" numbers here but I did not
want to change more than what was necessary.
Link: https://github.com/llvm/llvm-project/pull/82432 [1]
Closes: https://github.com/KSPP/linux/issues/357
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
Link: https://lore.kernel.org/r/20240508-b4-b4-sio-sr_select_speed-v2-1-00b68f724290@google.com
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
Documentation/cdrom/cdrom-standard.rst | 4 ++--
drivers/scsi/sr.h | 2 +-
drivers/scsi/sr_ioctl.c | 5 ++++-
include/linux/cdrom.h | 2 +-
4 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/Documentation/cdrom/cdrom-standard.rst b/Documentation/cdrom/cdrom-standard.rst
index 7964fe134277b..6c1303cff159e 100644
--- a/Documentation/cdrom/cdrom-standard.rst
+++ b/Documentation/cdrom/cdrom-standard.rst
@@ -217,7 +217,7 @@ current *struct* is::
int (*media_changed)(struct cdrom_device_info *, int);
int (*tray_move)(struct cdrom_device_info *, int);
int (*lock_door)(struct cdrom_device_info *, int);
- int (*select_speed)(struct cdrom_device_info *, int);
+ int (*select_speed)(struct cdrom_device_info *, unsigned long);
int (*get_last_session) (struct cdrom_device_info *,
struct cdrom_multisession *);
int (*get_mcn)(struct cdrom_device_info *, struct cdrom_mcn *);
@@ -396,7 +396,7 @@ action need be taken, and the return value should be 0.
::
- int select_speed(struct cdrom_device_info *cdi, int speed)
+ int select_speed(struct cdrom_device_info *cdi, unsigned long speed)
Some CD-ROM drives are capable of changing their head-speed. There
are several reasons for changing the speed of a CD-ROM drive. Badly
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 1175f2e213b56..dc899277b3a44 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -65,7 +65,7 @@ int sr_disk_status(struct cdrom_device_info *);
int sr_get_last_session(struct cdrom_device_info *, struct cdrom_multisession *);
int sr_get_mcn(struct cdrom_device_info *, struct cdrom_mcn *);
int sr_reset(struct cdrom_device_info *);
-int sr_select_speed(struct cdrom_device_info *cdi, int speed);
+int sr_select_speed(struct cdrom_device_info *cdi, unsigned long speed);
int sr_audio_ioctl(struct cdrom_device_info *, unsigned int, void *);
int sr_is_xa(Scsi_CD *);
diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
index fbdb5124d7f7d..7034b4126d421 100644
--- a/drivers/scsi/sr_ioctl.c
+++ b/drivers/scsi/sr_ioctl.c
@@ -422,11 +422,14 @@ int sr_reset(struct cdrom_device_info *cdi)
return 0;
}
-int sr_select_speed(struct cdrom_device_info *cdi, int speed)
+int sr_select_speed(struct cdrom_device_info *cdi, unsigned long speed)
{
Scsi_CD *cd = cdi->handle;
struct packet_command cgc;
+ /* avoid exceeding the max speed or overflowing integer bounds */
+ speed = clamp(0, speed, 0xffff / 177);
+
if (speed == 0)
speed = 0xffff; /* set to max */
else
diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index 67caa909e3e61..24fe410492006 100644
--- a/include/linux/cdrom.h
+++ b/include/linux/cdrom.h
@@ -76,7 +76,7 @@ struct cdrom_device_ops {
unsigned int clearing, int slot);
int (*tray_move) (struct cdrom_device_info *, int);
int (*lock_door) (struct cdrom_device_info *, int);
- int (*select_speed) (struct cdrom_device_info *, int);
+ int (*select_speed) (struct cdrom_device_info *, unsigned long);
int (*get_last_session) (struct cdrom_device_info *,
struct cdrom_multisession *);
int (*get_mcn) (struct cdrom_device_info *,
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 6.1 02/29] scsi: sr: Fix unintentional arithmetic wraparound
[not found] <20240618124018.3303162-1-sashal@kernel.org>
@ 2024-06-18 12:39 ` Sasha Levin
2024-06-18 12:39 ` [PATCH AUTOSEL 6.1 06/29] efi/libstub: zboot.lds: Discard .discard sections Sasha Levin
2024-06-18 12:39 ` [PATCH AUTOSEL 6.1 21/29] selftests/futex: pass _GNU_SOURCE without a value to the compiler Sasha Levin
2 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2024-06-18 12:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Justin Stitt, linux-hardening, Kees Cook, Martin K . Petersen,
Sasha Levin, phil, corbet, James.Bottomley, nathan, linux-doc,
linux-scsi, llvm
From: Justin Stitt <justinstitt@google.com>
[ Upstream commit 9fad9d560af5c654bb38e0b07ee54a4e9acdc5cd ]
Running syzkaller with the newly reintroduced signed integer overflow
sanitizer produces this report:
[ 65.194362] ------------[ cut here ]------------
[ 65.197752] UBSAN: signed-integer-overflow in ../drivers/scsi/sr_ioctl.c:436:9
[ 65.203607] -2147483648 * 177 cannot be represented in type 'int'
[ 65.207911] CPU: 2 PID: 10416 Comm: syz-executor.1 Not tainted 6.8.0-rc2-00035-gb3ef86b5a957 #1
[ 65.213585] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 65.219923] Call Trace:
[ 65.221556] <TASK>
[ 65.223029] dump_stack_lvl+0x93/0xd0
[ 65.225573] handle_overflow+0x171/0x1b0
[ 65.228219] sr_select_speed+0xeb/0xf0
[ 65.230786] ? __pm_runtime_resume+0xe6/0x130
[ 65.233606] sr_block_ioctl+0x15d/0x1d0
...
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 557f8c582a9b ("ubsan: Reintroduce signed overflow sanitizer").
Firstly, let's change the type of "speed" to unsigned long as
sr_select_speed()'s only caller passes in an unsigned long anyways.
$ git grep '\.select_speed'
| drivers/scsi/sr.c: .select_speed = sr_select_speed,
...
| static int cdrom_ioctl_select_speed(struct cdrom_device_info *cdi,
| unsigned long arg)
| {
| ...
| return cdi->ops->select_speed(cdi, arg);
| }
Next, let's add an extra check to make sure we don't exceed 0xffff/177
(350) since 0xffff is the max speed. This has two benefits: 1) we deal
with integer overflow before it happens and 2) we properly respect the
max speed of 0xffff. There are some "magic" numbers here but I did not
want to change more than what was necessary.
Link: https://github.com/llvm/llvm-project/pull/82432 [1]
Closes: https://github.com/KSPP/linux/issues/357
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
Link: https://lore.kernel.org/r/20240508-b4-b4-sio-sr_select_speed-v2-1-00b68f724290@google.com
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
Documentation/cdrom/cdrom-standard.rst | 4 ++--
drivers/scsi/sr.h | 2 +-
drivers/scsi/sr_ioctl.c | 5 ++++-
include/linux/cdrom.h | 2 +-
4 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/Documentation/cdrom/cdrom-standard.rst b/Documentation/cdrom/cdrom-standard.rst
index 7964fe134277b..6c1303cff159e 100644
--- a/Documentation/cdrom/cdrom-standard.rst
+++ b/Documentation/cdrom/cdrom-standard.rst
@@ -217,7 +217,7 @@ current *struct* is::
int (*media_changed)(struct cdrom_device_info *, int);
int (*tray_move)(struct cdrom_device_info *, int);
int (*lock_door)(struct cdrom_device_info *, int);
- int (*select_speed)(struct cdrom_device_info *, int);
+ int (*select_speed)(struct cdrom_device_info *, unsigned long);
int (*get_last_session) (struct cdrom_device_info *,
struct cdrom_multisession *);
int (*get_mcn)(struct cdrom_device_info *, struct cdrom_mcn *);
@@ -396,7 +396,7 @@ action need be taken, and the return value should be 0.
::
- int select_speed(struct cdrom_device_info *cdi, int speed)
+ int select_speed(struct cdrom_device_info *cdi, unsigned long speed)
Some CD-ROM drives are capable of changing their head-speed. There
are several reasons for changing the speed of a CD-ROM drive. Badly
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 1175f2e213b56..dc899277b3a44 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -65,7 +65,7 @@ int sr_disk_status(struct cdrom_device_info *);
int sr_get_last_session(struct cdrom_device_info *, struct cdrom_multisession *);
int sr_get_mcn(struct cdrom_device_info *, struct cdrom_mcn *);
int sr_reset(struct cdrom_device_info *);
-int sr_select_speed(struct cdrom_device_info *cdi, int speed);
+int sr_select_speed(struct cdrom_device_info *cdi, unsigned long speed);
int sr_audio_ioctl(struct cdrom_device_info *, unsigned int, void *);
int sr_is_xa(Scsi_CD *);
diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
index fbdb5124d7f7d..7034b4126d421 100644
--- a/drivers/scsi/sr_ioctl.c
+++ b/drivers/scsi/sr_ioctl.c
@@ -422,11 +422,14 @@ int sr_reset(struct cdrom_device_info *cdi)
return 0;
}
-int sr_select_speed(struct cdrom_device_info *cdi, int speed)
+int sr_select_speed(struct cdrom_device_info *cdi, unsigned long speed)
{
Scsi_CD *cd = cdi->handle;
struct packet_command cgc;
+ /* avoid exceeding the max speed or overflowing integer bounds */
+ speed = clamp(0, speed, 0xffff / 177);
+
if (speed == 0)
speed = 0xffff; /* set to max */
else
diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index 67caa909e3e61..24fe410492006 100644
--- a/include/linux/cdrom.h
+++ b/include/linux/cdrom.h
@@ -76,7 +76,7 @@ struct cdrom_device_ops {
unsigned int clearing, int slot);
int (*tray_move) (struct cdrom_device_info *, int);
int (*lock_door) (struct cdrom_device_info *, int);
- int (*select_speed) (struct cdrom_device_info *, int);
+ int (*select_speed) (struct cdrom_device_info *, unsigned long);
int (*get_last_session) (struct cdrom_device_info *,
struct cdrom_multisession *);
int (*get_mcn) (struct cdrom_device_info *,
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 6.1 06/29] efi/libstub: zboot.lds: Discard .discard sections
[not found] <20240618124018.3303162-1-sashal@kernel.org>
2024-06-18 12:39 ` [PATCH AUTOSEL 6.1 02/29] scsi: sr: Fix unintentional arithmetic wraparound Sasha Levin
@ 2024-06-18 12:39 ` Sasha Levin
2024-06-18 12:39 ` [PATCH AUTOSEL 6.1 21/29] selftests/futex: pass _GNU_SOURCE without a value to the compiler Sasha Levin
2 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2024-06-18 12:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Nathan Chancellor, Huacai Chen, Ard Biesheuvel, Sasha Levin,
chenhuacai, linux-efi, llvm
From: Nathan Chancellor <nathan@kernel.org>
[ Upstream commit 5134acb15d9ef27aa2b90aad46d4e89fcef79fdc ]
When building ARCH=loongarch defconfig + CONFIG_UNWINDER_ORC=y using
LLVM, there is a warning from ld.lld when linking the EFI zboot image
due to the use of unreachable() in number() in vsprintf.c:
ld.lld: warning: drivers/firmware/efi/libstub/lib.a(vsprintf.stub.o):(.discard.unreachable+0x0): has non-ABS relocation R_LARCH_32_PCREL against symbol ''
If the compiler cannot eliminate the default case for any reason, the
.discard.unreachable section will remain in the final binary but the
entire point of any section prefixed with .discard is that it is only
used at compile time, so it can be discarded via /DISCARD/ in a linker
script. The asm-generic vmlinux.lds.h includes .discard and .discard.*
in the COMMON_DISCARDS macro but that is not used for zboot.lds, as it
is not a kernel image linker script.
Add .discard and .discard.* to /DISCARD/ in zboot.lds, so that any
sections meant to be discarded at link time are not included in the
final zboot image. This issue is not specific to LoongArch, it is just
the first architecture to select CONFIG_OBJTOOL, which defines
annotate_unreachable() as an asm statement to add the
.discard.unreachable section, and use the EFI stub.
Closes: https://github.com/ClangBuiltLinux/linux/issues/2023
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Acked-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/firmware/efi/libstub/zboot.lds | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/firmware/efi/libstub/zboot.lds b/drivers/firmware/efi/libstub/zboot.lds
index 93d33f68333b2..a7fffbad6d46a 100644
--- a/drivers/firmware/efi/libstub/zboot.lds
+++ b/drivers/firmware/efi/libstub/zboot.lds
@@ -34,6 +34,7 @@ SECTIONS
}
/DISCARD/ : {
+ *(.discard .discard.*)
*(.modinfo .init.modinfo)
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 6.1 21/29] selftests/futex: pass _GNU_SOURCE without a value to the compiler
[not found] <20240618124018.3303162-1-sashal@kernel.org>
2024-06-18 12:39 ` [PATCH AUTOSEL 6.1 02/29] scsi: sr: Fix unintentional arithmetic wraparound Sasha Levin
2024-06-18 12:39 ` [PATCH AUTOSEL 6.1 06/29] efi/libstub: zboot.lds: Discard .discard sections Sasha Levin
@ 2024-06-18 12:39 ` Sasha Levin
2 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2024-06-18 12:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: John Hubbard, Edward Liaw, Davidlohr Bueso, Shuah Khan,
Sasha Levin, tglx, mingo, shuah, nathan, linux-kselftest, llvm
From: John Hubbard <jhubbard@nvidia.com>
[ Upstream commit cb708ab9f584f159798b60853edcf0c8b67ce295 ]
It's slightly better to set _GNU_SOURCE in the source code, but if one
must do it via the compiler invocation, then the best way to do so is
this:
$(CC) -D_GNU_SOURCE=
...because otherwise, if this form is used:
$(CC) -D_GNU_SOURCE
...then that leads the compiler to set a value, as if you had passed in:
$(CC) -D_GNU_SOURCE=1
That, in turn, leads to warnings under both gcc and clang, like this:
futex_requeue_pi.c:20: warning: "_GNU_SOURCE" redefined
Fix this by using the "-D_GNU_SOURCE=" form.
Reviewed-by: Edward Liaw <edliaw@google.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
tools/testing/selftests/futex/functional/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/futex/functional/Makefile b/tools/testing/selftests/futex/functional/Makefile
index a392d0917b4e5..994fa3468f170 100644
--- a/tools/testing/selftests/futex/functional/Makefile
+++ b/tools/testing/selftests/futex/functional/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
INCLUDES := -I../include -I../../ $(KHDR_INCLUDES)
-CFLAGS := $(CFLAGS) -g -O2 -Wall -D_GNU_SOURCE -pthread $(INCLUDES) $(KHDR_INCLUDES)
+CFLAGS := $(CFLAGS) -g -O2 -Wall -D_GNU_SOURCE= -pthread $(INCLUDES) $(KHDR_INCLUDES)
LDLIBS := -lpthread -lrt
LOCAL_HDRS := \
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-06-18 12:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240618124018.3303162-1-sashal@kernel.org>
2024-06-18 12:39 ` [PATCH AUTOSEL 6.1 02/29] scsi: sr: Fix unintentional arithmetic wraparound Sasha Levin
2024-06-18 12:39 ` [PATCH AUTOSEL 6.1 06/29] efi/libstub: zboot.lds: Discard .discard sections Sasha Levin
2024-06-18 12:39 ` [PATCH AUTOSEL 6.1 21/29] selftests/futex: pass _GNU_SOURCE without a value to the compiler Sasha Levin
[not found] <20240617132456.2588952-1-sashal@kernel.org>
2024-06-17 13:24 ` [PATCH AUTOSEL 6.1 02/29] scsi: sr: Fix unintentional arithmetic wraparound Sasha Levin
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).