On Mon, Aug 22, 2016 at 3:53 PM, Tom Yan <tom.ty89@xxxxxxxxx> wrote:
> On 22 August 2016 at 20:32, Shaun Tancheff <shaun.tancheff@xxxxxxxxxxx> wrote:
>> On Mon, Aug 22, 2016 at 3:07 PM, Tom Yan <tom.ty89@xxxxxxxxx> wrote:
>>> I don't see how that's possible. count / n_block will always be
>>> smaller than 65535 * ATA_MAX_TRIM_RNUM(64) = 4194240. Not to mention
>>> that isn't even a "buffer limit" anyway. By SG_IO do you mean like
>>> SCSI Write Same commands that issued with sg_write_same or so? If
>>> that's the case, that's what exactly commit 5c79097a28c2
>>> ("libata-scsi: reject WRITE SAME (16) with n_block that exceeds
>>> limit") is for.
>>
>> Ah, I see. You are guarding the only user of ata_set_lba_range_entries().
>
> Yup. It is the only right thing to do anyway, that we leave the
> function "open" and guard per context when we use it. Say if
> ata_set_lba_range_entries() is gonna be a function that is shared by
> others, it would only make this commit more important. As I said, we
> did not guard it with a certain buffer limit, but merely redundantly
> guard it with a ("humanized") limit that applies to TRIM only.
But the "humanized" limit is the one you just added and proceeded to
change ata_set_lba_range_entries(). You changed from a buffer size
to use "num" instead and now you want to remove the protection
entirely?
Why not just change to put this in front of ata_set_lba_range_entries()
if (n_block > 65535 * ATA_MAX_TRIM_RNUM) {
fp = 2;
goto invalid_fld;
}
And then restore ata_set_lba_range_entries() to how it looked
before you changed it in commit:
2983860c7 (libata-scsi: avoid repeated calculation of number of TRIM ranges)
Then you can have ata_set_lba_range_entries() take the buffer size ...
something like the following would be fine:
size = ata_set_lba_range_entries(buf, scmd->device->sector_size,
block, n_block);
Now things are happily protected against both exceeding the b0 limit(s) and
overflowing the sglist buffer.
--
Shaun
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
