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 557f8c582a ("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>
When a host is configured with a few LUNs and I/O is running, injecting FC
faults repeatedly leads to path recovery problems. The LUNs have 4 paths
each and 3 of them come back active after say an FC fault which makes 2 of
the paths go down, instead of all 4. This happens after several iterations
of continuous FC faults.
Reason here is that we're returning an I/O error whenever we're
encountering sense code 06/04/0a (LOGICAL UNIT NOT ACCESSIBLE, ASYMMETRIC
ACCESS STATE TRANSITION) instead of retrying.
[mwilck: The original patch was developed by Rajashekhar M A and Hannes
Reinecke. I moved the code to alua_check_sense() as suggested by Mike
Christie [1]. Evan Milne had raised the question whether pg->state should
be set to transitioning in the UA case [2]. I believe that doing this is
correct. SCSI_ACCESS_STATE_TRANSITIONING by itself doesn't cause I/O
errors. Our handler schedules an RTPG, which will only result in an I/O
error condition if the transitioning timeout expires.]
[1] https://lore.kernel.org/all/0bc96e82-fdda-4187-148d-5b34f81d4942@oracle.com/
[2] https://lore.kernel.org/all/CAGtn9r=kicnTDE2o7Gt5Y=yoidHYD7tG8XdMHEBJTBraVEoOCw@mail.gmail.com/
Co-developed-by: Rajashekhar M A <rajs@netapp.com>
Co-developed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Martin Wilck <martin.wilck@suse.com>
Link: https://lore.kernel.org/r/20240514140344.19538-1-mwilck@suse.com
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
When building for a 32-bit platform such as ARM or i386, for which size_t
is unsigned int, there is a warning due to using an unsigned long format
specifier:
drivers/scsi/mpi3mr/mpi3mr_transport.c:1370:11: error: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Werror,-Wformat]
1369 | ioc_warn(mrioc, "skipping port %u, max allowed value is %lu\n",
| ~~~
| %u
1370 | i, sizeof(mr_sas_port->phy_mask) * 8);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Use the proper format specifier for size_t, %zu, to resolve the warning for
all platforms.
Fixes: 3668651def ("scsi: mpi3mr: Sanitise num_phys")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Link: https://lore.kernel.org/r/20240514-mpi3mr-fix-wformat-v1-1-f1ad49217e5e@kernel.org
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The driver's I/O path is updated to support 32 byte CDBs.
Changes to accommodate 32 byte CDBs include:
- Updating various size fields to allow for the larger 32 byte CDB.
- Starting the FCP command payload at an earlier offset in WQE submission
to fit the 32 byte CDB.
- Redefining relevant structs to __le32/__be32 data types for proper cpu
endianness macro usage.
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Link: https://lore.kernel.org/r/20240429221547.6842-7-justintee8345@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Device recovery logic is skipped when the RSCN processing flag is set.
However during rmmod, the flag is not cleared leading to unnecessary delays
in waiting for completions on a link that is being offlined.
Move clearing of the RSCN deferred flag to a refactored routine when called
from device recovery, and set the IA flag when issuing an abort during
unload.
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Link: https://lore.kernel.org/r/20240429221547.6842-4-justintee8345@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Peter Griffin <peter.griffin@linaro.org> says:
Hi Martin, James & Alim,
This series adds support to the ufs-exynos driver for Tensor gs101
found in Pixel 6. It was send previously in [1] and [2] but included
the other clock, phy and DTS parts. This series has been split into
just the ufs-exynos part to hopefully make things easier.
With this series, plus the phy, clock and dts changes UFS is
functional upstream for Pixel 6. The SKhynix HN8T05BZGKX015 can be
enumerated, partitions mounted etc.
The series is split into some prepatory patches for ufs-exynos and a
final patch that adds the gs101 support.
Note the sysreg clock has been moved to ufs node as fine grained clock
control around the syscon sysreg register accesses doesn't result in
functional UFS.
regards,
Peter
Link: https://lore.kernel.org/r/20240426122004.2249178-1-peter.griffin@linaro.org
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Add a dedicated compatible and drv_data with associated hooks for gs101 SoC
found on Pixel 6.
Note we make use of the previously added EXYNOS_UFS_OPT_UFSPR_SECURE
option, to skip initialisation of UFSPR registers as these are only
accessible via SMC call.
EXYNOS_UFS_OPT_TIMER_TICK_SELECT option is also set to select tick
source. This has been done so as not to effect any existing platforms.
DBG_OPTION_SUITE on gs101 has different address offsets to other SoCs so
these register offsets now come from uic_attr struct.
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Link: https://lore.kernel.org/r/20240426122004.2249178-7-peter.griffin@linaro.org
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
Tested-by: Will McVicker <willmcvicker@google.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
DebugFS output for fw_resource_count shows:
estimate exchange used[0] high water limit [1945] n estimate iocb2 used [0] high water limit [5141]
estimate exchange2 used[0] high water limit [1945]
Which shows incorrect display due to missing newline in seq_print().
[mkp: fix checkpatch warning about space before newline]
Fixes: 5f63a163ed ("scsi: qla2xxx: Fix exchange oversubscription for management commands")
Signed-off-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Link: https://lore.kernel.org/r/20240426020056.3639406-1-himanshu.madhani@oracle.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Currently, we allocate a count-sized kernel buffer and copy count from
userspace to that buffer. Later, we use kstrtouint on this buffer but we
don't ensure that the string is terminated inside the buffer, this can
lead to OOB read when using kstrtouint. Fix this issue by using
memdup_user_nul instead of memdup_user.
Fixes: 61d8658b4a ("scsi: qedf: Add QLogic FastLinQ offload FCoE driver framework.")
Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
Link: https://lore.kernel.org/r/20240424-fix-oob-read-v2-4-f1f1b53a10f4@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Currently, we allocate a nbytes-sized kernel buffer and copy nbytes from
userspace to that buffer. Later, we use sscanf on this buffer but we don't
ensure that the string is terminated inside the buffer, this can lead to
OOB read when using sscanf. Fix this issue by using memdup_user_nul instead
of memdup_user.
Fixes: 9f30b67475 ("bfa: replace 2 kzalloc/copy_from_user by memdup_user")
Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
Link: https://lore.kernel.org/r/20240424-fix-oob-read-v2-3-f1f1b53a10f4@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Correct the name of a struct in kernel-doc to match the actual function
name.
Add kernel-doc comments for 2 reserved fields to match comments for other
reserved fields.
Correct the kernel-doc comments for a nested struct to eliminate kernel-doc
warnings for them.
Warnings fixed here are:
scsi_bsg_mpi3mr.h:419: warning: expecting prototype for struct mpi3mr_bsg_buf_entry_list. Prototype was for struct mpi3mr_buf_entry_list instead
scsi_bsg_mpi3mr.h:435: warning: Function parameter or struct member 'rsvd2' not described in 'mpi3mr_bsg_mptcmd'
scsi_bsg_mpi3mr.h:456: warning: Function parameter or struct member 'rsvd3' not described in 'mpi3mr_bsg_packet'
scsi_bsg_mpi3mr.h:456: warning: Excess struct member 'drvrcmd' description in 'mpi3mr_bsg_packet'
scsi_bsg_mpi3mr.h:456: warning: Excess struct member 'mptcmd' description in 'mpi3mr_bsg_packet'
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Link: https://lore.kernel.org/r/20240424055322.1400-1-rdunlap@infradead.org
Cc: James E.J. Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Sathya Prakash Veerichetty <sathya.prakash@broadcom.com>
Cc: Kashyap Desai <kashyap.desai@broadcom.com>
Cc: Sumit Saxena <sumit.saxena@broadcom.com>
Cc: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
Cc: mpi3mr-linuxdrv.pdl@broadcom.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Complete the kernel-doc notation for enum fc_lport_state. This fixes 7
kernel-doc warnings.
- In struct fc_rport_priv, change 'event_callback' to 'lld_event_callback'
to match the struct member name.
- In struct fc_fcp_pkt, add a description for 'timer_delay' to eliminate
one kernel-doc warning.
- Add return value notation for 3 functions. This fixes 3 kernel-doc
warnings.
There are still 12 warnings for struct members not described in struct
fc_rport_priv and struct fc_lport, e.g:
libfc.h:218: warning: Function parameter or struct member 'event' not described in 'fc_rport_priv'
libfc.h:760: warning: Function parameter or struct member 'vlan' not described in 'fc_lport'
Warnings that are fixed in this patch:
libfc.h:75: warning: Enum value 'LPORT_ST_RNN_ID' not described in enum 'fc_lport_state'
libfc.h:75: warning: Enum value 'LPORT_ST_RSNN_NN' not described in enum 'fc_lport_state'
libfc.h:75: warning: Enum value 'LPORT_ST_RSPN_ID' not described in enum 'fc_lport_state'
libfc.h:75: warning: Enum value 'LPORT_ST_RPA' not described in enum 'fc_lport_state'
libfc.h:75: warning: Enum value 'LPORT_ST_DHBA' not described in enum 'fc_lport_state'
libfc.h:75: warning: Enum value 'LPORT_ST_DPRT' not described in enum 'fc_lport_state'
libfc.h:75: warning: Excess enum value 'LPORT_ST_RPN_ID' description in 'fc_lport_state'
libfc.h:218: warning: Excess struct member 'event_callback' description in 'fc_rport_priv'
libfc.h:793: warning: No description found for return value of 'fc_lport_test_ready'
libfc.h:835: warning: No description found for return value of 'fc_lport_init_stats'
libfc.h:856: warning: No description found for return value of 'lport_priv'
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Link: https://lore.kernel.org/r/20240424050038.31403-1-rdunlap@infradead.org
Cc: Hannes Reinecke <hare@suse.de>
Cc: James E.J. Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The variable 'i' is being assigned a value that is never read, the
following code path via the label ofld_err never refers to the
variable. The assignment is redundant and can be removed.
Cleans up clang scan warning:
drivers/scsi/bnx2fc/bnx2fc_tgt.c:132:5: warning: Value stored to 'i'
is never read [deadcode.DeadStores]
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Link: https://lore.kernel.org/r/20240415104311.484890-1-colin.i.king@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Fix the calculation of the utrd pointer. This patch addresses the following
Coverity complaint:
CID 1538170: (#1 of 1): Extra sizeof expression (SIZEOF_MISMATCH)
suspicious_pointer_arithmetic: Adding sq_head_slot * 32UL /* sizeof (struct
utp_transfer_req_desc) */ to pointer hwq->sqe_base_addr of type struct
utp_transfer_req_desc * is suspicious because adding an integral value to
this pointer automatically scales that value by the size, 32 bytes, of the
pointed-to type, struct utp_transfer_req_desc. Most likely, the
multiplication by sizeof (struct utp_transfer_req_desc) in this expression
is extraneous and should be eliminated.
Cc: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Can Guo <quic_cang@quicinc.com>
Fixes: 8d72903489 ("scsi: ufs: mcq: Add supporting functions for MCQ abort")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Link: https://lore.kernel.org/r/20240410000751.1047758-1-bvanassche@acm.org
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
struct Scsi_Host private data contains pointer to struct ctlr_info.
Restore allocation of only 8 bytes to store pointer in struct Scsi_Host
private data area.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: bbbd254991 ("scsi: hpsa: Fix allocation size for scsi_host_alloc()")
Signed-off-by: Yuri Karpov <YKarpov@ispras.ru>
Link: https://lore.kernel.org/r/20240312170447.743709-1-YKarpov@ispras.ru
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
As of commit 7d1d865181 ("[SCSI] libsas: fix false positive 'device
attached' conditions"), reset the phy->entacted_sas_addr address to a
zero-address when the link rate is less than 1.5G.
Currently we find that when a new device is attached, and the link rate is
less than 1.5G, but the device type is not NO_DEVICE, for example: the link
rate is SAS_PHY_RESET_IN_PROGRESS and the device type is stp. After setting
the phy->entacted_sas_addr address to the zero address, the port will
continue to be created for the phy with the zero-address, and other phys
with the zero-address will be tried to be added to the new port:
[562240.051197] sas: ex 500e004aaaaaaa1f phy19:U:0 attached: 0000000000000000 (no device)
// phy19 is deleted but still on the parent port's phy_list
[562240.062536] sas: ex 500e004aaaaaaa1f phy0 new device attached
[562240.062616] sas: ex 500e004aaaaaaa1f phy00:U:5 attached: 0000000000000000 (stp)
[562240.062680] port-7:7:0: trying to add phy phy-7:7:19 fails: it's already part of another port
Therefore, it should be the same as sas_get_phy_attached_dev(). Only when
device_type is SAS_PHY_UNUSED, sas_address is set to the 0 address.
Fixes: 7d1d865181 ("[SCSI] libsas: fix false positive 'device attached' conditions")
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
Link: https://lore.kernel.org/r/20240312141103.31358-5-yangxingui@huawei.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
We found that when ex_phy was attached and added to the parent wide port,
ex_phy->port was not set, resulting in sas_unregister_devs_sas_addr() not
calling sas_port_delete_phy() when deleting the phy, and the deleted phy
was still on the parent wide port's phy_list.
When we use sas_port_add_ex_phy() to set ex_phy->port to solve the above
problem, we find that after all the phys of the parent_port are removed and
the number of phy becomes 0, the parent_port will not be set to NULL. This
causes the freed parent port to be used when attaching a new ex_phy in
sas_ex_add_parent_port().
Use sas_port_add_ex_phy() instead of sas_port_add_phy() to set ex_phy->port
when ex_phy is added to the parent port, and set ex_dev->parent_port to
NULL when the number of phy on the port becomes 0.
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
Link: https://lore.kernel.org/r/20240312141103.31358-4-yangxingui@huawei.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Christoph Hellwig <hch@lst.de> says:
Hi all,
this series converts the SCSI midlayer and LLDDs to use atomic queue
limits API. It is pretty straight forward, except for the mpt3mr
driver which does really weird and probably already broken things by
setting limits from unlocked device iteration callbacks.
I will probably defer the (more complicated) ULD changes to the next
merge window as they would heavily conflict with Damien's zone write
plugging series. With that the series could go in through the SCSI
tree if Jens' ACKs the core block layer bits.
Link: https://lore.kernel.org/r/20240409143748.980206-1-hch@lst.de
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Switch to the ->device_configure method instead of ->slave_alloc and update
the block limits on the passed in queue_limits instead of using the
per-limit accessors.
Note that uas was the only driver setting these size limits from
->slave_alloc and not ->slave_configure and this makes it match everyone
else.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20240409143748.980206-23-hch@lst.de
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The cxlflash driver is no longer actively maintained and we intend to
remove it in a future kernel release. Change its status to obsolete.
While we're here, Matthew Ochs no longer works at IBM and is no longer in a
position to access cxlflash hardware, so remove him from the maintainers
list.
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
Link: https://lore.kernel.org/r/20240409031027.41587-1-ajd@linux.ibm.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
