linux-kernel - Re: [PATCH] nvme: clear the prp2 field of the nvme command.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 29 Nov 2022 12:50:57 +0800
From:   "Rao, Lei" <lei.rao@...el.com>
To:     Chaitanya Kulkarni <chaitanyak@...dia.com>,
        "kbusch@...nel.org" <kbusch@...nel.org>,
        "axboe@...com" <axboe@...com>, "hch@....de" <hch@....de>,
        "sagi@...mberg.me" <sagi@...mberg.me>,
        "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] nvme: clear the prp2 field of the nvme command.



On 11/29/2022 12:16 PM, Chaitanya Kulkarni wrote:
> On 11/28/22 17:47, Lei Rao wrote:
>> If the prp2 field is not filled in nvme_setup_prp_simple(), the prp2
>> field is garbage data. According to nvme spec, the prp2 is reserved if
>> the data transfer does not cross a memory page boundary. Writing a
>> reserved coded value into a controller property field produces undefined
>> results, so it needs to be cleared in nvme_setup_rw().
>>
>> Signed-off-by: Lei Rao <lei.rao@...el.com>
> 
> if it is reserved then controller shoule ignore this field, no ?

It's feasible for the controller to ignore this field. But our controller has
stricter checks, and if prp2 is not used but has a value, some warnings will be
printed. According to the NVMe spec, it seems to write a reserved field produces
an undefined result, so maybe clearing it is better.

Thanks,
Lei

> 
> not sure if original author wanted to avoid an extra assignment
> in the fast path with assumption that reserved fields should be
> ignored if it is then we should avoid this, if not then looks good
> 
> Reviewed-by: Chaitanya Kulkarni <kch@...dia.com>
> 
> -ck
>