mbox series

[v2,0/6] Ensure the copied buf is NUL terminated

Message ID 20240424-fix-oob-read-v2-0-f1f1b53a10f4@gmail.com
Headers show
Series Ensure the copied buf is NUL terminated | expand

Message

Bui Quang Minh April 24, 2024, 2:44 p.m. UTC
Hi everyone,

I found that some drivers contains an out-of-bound read pattern like this

	kern_buf = memdup_user(user_buf, count);
	...
	sscanf(kern_buf, ...);

The sscanf can be replaced by some other string-related functions. This
pattern can lead to out-of-bound read of kern_buf in string-related
functions.

This series fix the above issue by replacing memdup_user with
memdup_user_nul.

Thanks,
Quang Minh.

To: Jesse Brandeburg <jesse.brandeburg@intel.com>
To: Tony Nguyen <anthony.l.nguyen@intel.com>
To: David S. Miller <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
To: Rasesh Mody <rmody@marvell.com>
To: Sudarsana Kalluru <skalluru@marvell.com>
To: GR-Linux-NIC-Dev@marvell.com
To: Anil Gurumurthy <anil.gurumurthy@qlogic.com>
To: Sudarsana Kalluru <sudarsana.kalluru@qlogic.com>
To: James E.J. Bottomley <James.Bottomley@HansenPartnership.com>
To: Martin K. Petersen <martin.petersen@oracle.com>
To: Fabian Frederick <fabf@skynet.be>
To: Saurav Kashyap <skashyap@marvell.com>
To: GR-QLogic-Storage-Upstream@marvell.com
To: Nilesh Javali <nilesh.javali@cavium.com>
To: Arun Easi <arun.easi@cavium.com>
To: Manish Rangankar <manish.rangankar@cavium.com>
To: Vineeth Vijayan <vneethv@linux.ibm.com>
To: Peter Oberparleiter <oberpar@linux.ibm.com>
To: Heiko Carstens <hca@linux.ibm.com>
To: Vasily Gorbik <gor@linux.ibm.com>
To: Alexander Gordeev <agordeev@linux.ibm.com>
To: Christian Borntraeger <borntraeger@linux.ibm.com>
To: Sven Schnelle <svens@linux.ibm.com>
To: Dupuis, Chad <chad.dupuis@cavium.com>
To: Sunil Goutham <sgoutham@marvell.com>
To: Linu Cherian <lcherian@marvell.com>
To: Geetha sowjanya <gakula@marvell.com>
To: Jerin Jacob <jerinj@marvell.com>
To: hariprasad <hkelam@marvell.com>
To: Subbaraya Sundeep <sbhatta@marvell.com>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: Saurav Kashyap <saurav.kashyap@cavium.com>
Cc: linux-s390@vger.kernel.org
Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>

Changes in v2:
- Patch 5: use memdup_user_nul instead
- Add patch 6
- Link to v1: https://lore.kernel.org/r/20240422-fix-oob-read-v1-0-e02854c30174@gmail.com

---
Bui Quang Minh (6):
      ice: ensure the copied buf is NUL terminated
      bna: ensure the copied buf is NUL terminated
      bfa: ensure the copied buf is NUL terminated
      qedf: ensure the copied buf is NUL terminated
      cio: ensure the copied buf is NUL terminated
      octeontx2-af: avoid off-by-one read from userspace

 drivers/net/ethernet/brocade/bna/bnad_debugfs.c         | 4 ++--
 drivers/net/ethernet/intel/ice/ice_debugfs.c            | 8 ++++----
 drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c | 4 +---
 drivers/s390/cio/cio_inject.c                           | 2 +-
 drivers/scsi/bfa/bfad_debugfs.c                         | 4 ++--
 drivers/scsi/qedf/qedf_debugfs.c                        | 2 +-
 6 files changed, 11 insertions(+), 13 deletions(-)
---
base-commit: ed30a4a51bb196781c8058073ea720133a65596f
change-id: 20240422-fix-oob-read-19ae7f8f3711

Best regards,

Comments

patchwork-bot+netdevbpf@kernel.org April 26, 2024, 2:30 a.m. UTC | #1
Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 24 Apr 2024 21:44:17 +0700 you wrote:
> Hi everyone,
> 
> I found that some drivers contains an out-of-bound read pattern like this
> 
> 	kern_buf = memdup_user(user_buf, count);
> 	...
> 	sscanf(kern_buf, ...);
> 
> [...]

Here is the summary with links:
  - [v2,1/6] ice: ensure the copied buf is NUL terminated
    https://git.kernel.org/netdev/net/c/666854ea9cad
  - [v2,2/6] bna: ensure the copied buf is NUL terminated
    https://git.kernel.org/netdev/net/c/8c34096c7fdf
  - [v2,3/6] bfa: ensure the copied buf is NUL terminated
    (no matching commit)
  - [v2,4/6] qedf: ensure the copied buf is NUL terminated
    (no matching commit)
  - [v2,5/6] cio: ensure the copied buf is NUL terminated
    (no matching commit)
  - [v2,6/6] octeontx2-af: avoid off-by-one read from userspace
    https://git.kernel.org/netdev/net/c/f299ee709fb4

You are awesome, thank you!
Alexander Gordeev April 26, 2024, 10:10 a.m. UTC | #2
On Wed, Apr 24, 2024 at 05:16:56PM +0200, Alexander Gordeev wrote:
> Applied, thanks!

Hi Jakub,

I just want to make sure you do not have plans to pull this patch
via the net tree, right? (I schedulled it for the s390 tree already).

Thanks!
Jakub Kicinski April 26, 2024, 2:29 p.m. UTC | #3
On Fri, 26 Apr 2024 12:10:35 +0200 Alexander Gordeev wrote:
> On Wed, Apr 24, 2024 at 05:16:56PM +0200, Alexander Gordeev wrote:
> > Applied, thanks!  
> 
> Hi Jakub,
> 
> I just want to make sure you do not have plans to pull this patch
> via the net tree, right? (I schedulled it for the s390 tree already).

Yes, go for it. I picked 1, 2 and 6, no interest in the other 3 :)
Martin K. Petersen May 7, 2024, 1:19 a.m. UTC | #4
Bui,

> 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.

Applied to 6.10/scsi-staging, thanks!
Martin K. Petersen May 11, 2024, 6:39 p.m. UTC | #5
On Wed, 24 Apr 2024 21:44:17 +0700, Bui Quang Minh wrote:

> I found that some drivers contains an out-of-bound read pattern like this
> 
> 	kern_buf = memdup_user(user_buf, count);
> 	...
> 	sscanf(kern_buf, ...);
> 
> The sscanf can be replaced by some other string-related functions. This
> pattern can lead to out-of-bound read of kern_buf in string-related
> functions.
> 
> [...]

Applied to 6.10/scsi-queue, thanks!

[3/6] bfa: ensure the copied buf is NUL terminated
      https://git.kernel.org/mkp/scsi/c/13d0cecb4626
[4/6] qedf: ensure the copied buf is NUL terminated
      https://git.kernel.org/mkp/scsi/c/d0184a375ee7