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

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 <[e-mail]>
To: Tony Nguyen <[e-mail]>
To: David S. Miller <[e-mail]>
To: Eric Dumazet <[e-mail]>
To: Jakub Kicinski <[e-mail]>
To: Paolo Abeni <[e-mail]>
To: Paul M Stillwell Jr <[e-mail]>
To: Rasesh Mody <[e-mail]>
To: Sudarsana Kalluru <[e-mail]>
To: [e-mail]
To: Anil Gurumurthy <[e-mail]>
To: Sudarsana Kalluru <[e-mail]>
To: James E.J. Bottomley <[e-mail]>
To: Martin K. Petersen <[e-mail]>
To: Fabian Frederick <[e-mail]>
To: Saurav Kashyap <[e-mail]>
To: [e-mail]
To: Nilesh Javali <[e-mail]>
To: Arun Easi <[e-mail]>
To: Manish Rangankar <[e-mail]>
To: Vineeth Vijayan <[e-mail]>
To: Peter Oberparleiter <[e-mail]>
To: Heiko Carstens <[e-mail]>
To: Vasily Gorbik <[e-mail]>
To: Alexander Gordeev <[e-mail]>
To: Christian Borntraeger <[e-mail]>
To: Sven Schnelle <[e-mail]>
To: Dupuis, Chad <[e-mail]>
To: Sunil Goutham <[e-mail]>
To: Linu Cherian <[e-mail]>
To: Geetha sowjanya <[e-mail]>
To: Jerin Jacob <[e-mail]>
To: hariprasad <[e-mail]>
To: Subbaraya Sundeep <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: Saurav Kashyap <[e-mail]>
Cc: [e-mail]
Cc: Jens Axboe <[e-mail]>
Signed-off-by: Bui Quang Minh <[e-mail]>

Changes in v2:
- Patch 5: use memdup_user_nul instead
- Add patch 6
- Link to v1: https://lore.kernel.org/r/[e-mail]

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

[e-mail] April 26, 2024, 2:30 a.m. UTC | #1

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <[e-mail]>:

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!

Martin K. Petersen May 11, 2024, 6:39 p.m. UTC | #2

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