linux-kernel - Re: [PATCH v2 2/2] cxl/cdat: Fix header sum value in CDAT checksum

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]

Message-ID: <20240108150919.000045d4@Huawei.com>
Date: Mon, 8 Jan 2024 15:09:19 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Ira Weiny <ira.weiny@...el.com>
CC: Fan Ni <nifan.cxl@...il.com>, Dave Jiang <dave.jiang@...el.com>,
	<linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>, Huai-Cheng Kuo
	<hchkuo@...ry-design.com.tw>
Subject: Re: [PATCH v2 2/2] cxl/cdat: Fix header sum value in CDAT checksum

On Tue, 19 Dec 2023 15:32:18 -0800
Ira Weiny <ira.weiny@...el.com> wrote:

> Jonathan Cameron wrote:
> > On Wed, 29 Nov 2023 17:33:04 -0800
> > Ira Weiny <ira.weiny@...el.com> wrote:
> >   
> > > The addition of the DCD support for CXL type-3 devices extended the CDAT
> > > table large enough that the checksum being returned was incorrect.[1]
> > > 
> > > This was because the checksum value was using the header length field
> > > rather than each of the 4 bytes of the length field.  This was
> > > previously not seen because the length of the CDAT data was less than
> > > 256 thus resulting in an equivalent checksum value.
> > > 
> > > Properly calculate the checksum for the CDAT header.
> > > 
> > > [1] https://lore.kernel.org/all/20231116-fix-cdat-devm-free-v1-1-b148b40707d7@intel.com/
> > > 
> > > Fixes: aba578bdace5 ("hw/cxl/cdat: CXL CDAT Data Object Exchange implementation")
> > > Cc: Huai-Cheng Kuo <hchkuo@...ry-design.com.tw>
> > > Signed-off-by: Ira Weiny <ira.weiny@...el.com>
> > > 
> > > ---
> > > Changes from V1:
> > > [djiang: Remove do {} while (0);]
> > > ---
> > >  hw/cxl/cxl-cdat.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c
> > > index 24829cf2428d..67e44a4f992a 100644
> > > --- a/hw/cxl/cxl-cdat.c
> > > +++ b/hw/cxl/cxl-cdat.c
> > > @@ -49,6 +49,7 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp)
> > >      g_autofree CDATTableHeader *cdat_header = NULL;
> > >      g_autofree CDATEntry *cdat_st = NULL;
> > >      uint8_t sum = 0;
> > > +    uint8_t *buf;  
> > This results in a shadowing variable warning. I'll rename it to hdr_buf or something
> > like that.  
> 
> <sigh>  sorry again...
> 
> With all the discussion did you want me to re-roll the set?
> 
> AFAICS this is the only actual issue.  So if you want to do it that would
> be great.
> 
I've done it locally - just not dealt with some other stuff on that tree yet hence
not pushed out.  Will get to that sometime this week hopefully.

Jonathan

> Thanks,
> Ira