Re: [PATCH] nvmet: fix heap out-of-bounds read in nvmet_auth_negotiate()

在 2026/8/5 00:19, Christoph Hellwig 写道:
> On Tue, Aug 04, 2026 at 11:38:00AM +0800, Guixin Liu wrote:
>> nvmet_execute_auth_send() allocates the DH-HMAC-CHAP message buffer with
>> the host-supplied transfer length (tl) and hands it to
>> nvmet_auth_negotiate() without passing tl along. nvmet_auth_negotiate()
>> then reads the negotiate header and, for each of the halen hash
>> identifiers and dhlen DH group identifiers, indexes into the fixed
>> idlist[60] array (hashes at idlist[0..halen), groups at idlist[30..]).
>>
>> Neither the transfer length nor halen/dhlen is validated. A malicious or
>> non-conformant host can report a tl smaller than the negotiate structure,
>> or a halen/dhlen larger than the array (both are u8, up to 255), making
>> the loops read past the end of the allocated buffer (heap out-of-bounds
>> read). The sibling nvmet_auth_reply() already validates tl against the
>> structure size; the negotiate path did not.
>>
>> Pass tl into nvmet_auth_negotiate(), reject a tl that does not cover the
>> negotiate data plus one full protocol descriptor, and reject halen/dhlen
>> larger than NVME_AUTH_DHCHAP_MAX_DH_IDS.
>>
>> Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
>> Signed-off-by: Guixin Liu <[email protected]>
>> ---
>>   drivers/nvme/target/fabrics-cmd-auth.c | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
>> index d1b39e64d877..d015968bae2d 100644
>> --- a/drivers/nvme/target/fabrics-cmd-auth.c
>> +++ b/drivers/nvme/target/fabrics-cmd-auth.c
>> @@ -31,12 +31,16 @@ void nvmet_auth_sq_init(struct nvmet_sq *sq)
>>   	sq->dhchap_step = NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE;
>>   }
>>   
>> -static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
>> +static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d, u32 tl)
>>   {
>>   	struct nvmet_ctrl *ctrl = req->sq->ctrl;
>>   	struct nvmf_auth_dhchap_negotiate_data *data = d;
>>   	int i, hash_id = 0, fallback_hash_id = 0, dhgid, fallback_dhgid;
>>   
>> +	if (tl < sizeof(*data) +
>> +		 sizeof(struct nvmf_auth_dhchap_protocol_descriptor))
> Odd formatting here.  Condition continuations either get two-tab indents
> or after the brace:
>
> 	if (tl < sizeof(*data) +
> 			sizeof(struct nvmf_auth_dhchap_protocol_descriptor))
>
> 	if (tl <
> 	   sizeof(*data) + sizeof(struct nvmf_auth_dhchap_protocol_descriptor))
Well, this is an oversight.

Hi Keith, could you please fix this when you apply?  Or I can send a v2 
patch.

Best Regards,
Guixin Liu
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <[email protected]>