From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from verein.lst.de (verein.lst.de [213.95.11.211]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C76FD3C0A09 for ; Mon, 1 Jun 2026 14:49:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.95.11.211 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780325375; cv=none; b=Jb6lJsOfZZjLbhX7Ozou4b45J/ASLOat620ddNYgJgzF90d/6nrYVDJIUfiVeKJcUNgr2lTVm1hBSc54zRktNl2TOvFI75U3Y0Apjg5LN2K6v0EiPYOgQgICUgY7OOFQ6Gv6en+Nk56zVAbChWOvxMZ9IWzeOxR7AbiFuC0h9tQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780325375; c=relaxed/simple; bh=4E8xRBHrF2vGvf6SGkIiI8KARVPRv5ZGhzr115AyDLU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=bjkMzMFGILcW0WUVBB4Dl2tga6Ef+satPIkD9Iq07JWVMqMrKyC7DGD25LnfVvtQHCmxgBag1rMRYumdOj1QYZxWYGEY1w0Ft0vaf9Uo9Anoxiktbaq1cpvffMboXxLPDBU0nzIjFu9rXOf5cOron0ungs4rlUM2zM1komsooMw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=lst.de; spf=pass smtp.mailfrom=lst.de; arc=none smtp.client-ip=213.95.11.211 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=lst.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=lst.de Received: by verein.lst.de (Postfix, from userid 2407) id 8170868BFE; Mon, 1 Jun 2026 16:49:29 +0200 (CEST) Date: Mon, 1 Jun 2026 16:49:29 +0200 From: Christoph Hellwig To: Tianchu Chen Cc: Christoph Hellwig , hare@suse.de, sagi@grimberg.me, kch@nvidia.com, linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org Subject: Re: [PATCH] nvmet-auth: validate reply message payload bounds against transfer length Message-ID: <20260601144929.GB5054@lst.de> References: <20260601071921.GB7582@lst.de> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17 (2007-11-01) On Mon, Jun 01, 2026 at 08:58:18AM +0000, Tianchu Chen wrote: > I think the current patch doesn't actually allows overflow. > > hl is a __u8 and dhvlen is a __le16. So the maximum value of > sizeof(*data) + 2 * (size_t)data->hl + dhvlen > is 66061, which is far below SIZE_MAX. It can't wrap. True. > > About rewritting using struct_size: we still need to check > tl >= struct_size(data, rval, 0) first, Yes, or the existing sizeof. > which is exactly the same > as just using sizeof(*data) in current patch. Because we have to > confirm the header is fully present before we can even read > hl / dhvlen, which are the values that determine the minimum > required rval[] length. > > So I believe the current patch already achieves the intended > goal of validating the payload against the buffer size, > using struct_size won't change the behavior. I find untangling magic numbers a bit hard. Now unlike more conventional cases we're stuck with some of them, but trying to make the expression as self-describing as possible would still be nice. > That said, I'm happy to send a v2 that uses struct_size(data, rval, 0) For the anchor that's not all that useful. Anyway, let's keep the original version for now. I noticed there is more of that 2 * magic in surrounding code, so we'll need to address that separately instead of burdening it on you.