All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <cassel@kernel.org>
To: Damien Le Moal <dlemoal@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>, Christoph Hellwig <hch@lst.de>,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	reveliofuzzing <reveliofuzzing@gmail.com>,
	linux-ide@vger.kernel.org
Subject: Re: [PATCH] ata: libata-sff: Ensure that we cannot write outside the allocated buffer
Date: Mon, 27 Jan 2025 08:38:49 +0100	[thread overview]
Message-ID: <Z5c4CZnLNxlasYpe@ryzen> (raw)
In-Reply-To: <4dee6487-4b9a-408c-aa7c-834802781887@kernel.org>

On Sat, Jan 25, 2025 at 08:12:38AM +0900, Damien Le Moal wrote:
> On 1/24/25 23:11, Niklas Cassel wrote:
> > reveliofuzzing reported that a SCSI_IOCTL_SEND_COMMAND ioctl with out_len
> > set to 0xd42, SCSI command set to ATA_16 PASS-THROUGH, ATA command set to
> > ATA_NOP, and protocol set to ATA_PROT_PIO, can cause ata_pio_sector() to
> > write outside the allocated buffer, overwriting random memory.
> > 
> > While a ATA device is supposed to abort a ATA_NOP command, there does seem
> > to be a bug either in libata-sff or QEMU, where either this status is not
> > set, or the status is cleared before read by ata_sff_hsm_move().
> > Anyway, that is most likely a separate bug.
> > 
> > Looking at __atapi_pio_bytes(), it already has a safety check to ensure
> > that __atapi_pio_bytes() cannot write outside the allocated buffer.
> > 
> > Add a similar check to ata_pio_sector(), such that also ata_pio_sector()
> > cannot write outside the allocated buffer.
> > 
> > Reported-by: reveliofuzzing <reveliofuzzing@gmail.com>
> > Closes: https://lore.kernel.org/linux-ide/CA+-ZZ_jTgxh3bS7m+KX07_EWckSnW3N2adX3KV63y4g7M4CZ2A@mail.gmail.com/
> > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> 
> Looks good to me. But doesn't this need Fixes + Cc-stable tags ?

ata_pio_sector() has been able to write more data than what fits in the
buffer since the commit that imported linux into git:
1da177e4c3f4 ("Linux-2.6.12-rc2")

although ata_pio_sector() then lived in: drivers/scsi/libata-core.c

Do you want me to use this as the Fixes tag?


Kind regards,
Niklas

      reply	other threads:[~2025-01-27  7:38 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-24 14:11 [PATCH] ata: libata-sff: Ensure that we cannot write outside the allocated buffer Niklas Cassel
2025-01-24 23:12 ` Damien Le Moal
2025-01-27  7:38   ` Niklas Cassel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z5c4CZnLNxlasYpe@ryzen \
    --to=cassel@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=dlemoal@kernel.org \
    --cc=hch@lst.de \
    --cc=linux-ide@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=reveliofuzzing@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.