linux-kernel - Re: [PATCH] cxl/hdm: Fix hdm decoder init by adding COMMIT field check

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: <640276695c8e8_5b27929473@dwillia2-xfh.jf.intel.com.notmuch>
Date:   Fri, 3 Mar 2023 14:36:25 -0800
From:   Dan Williams <dan.j.williams@...el.com>
To:     Fan Ni <fan.ni@...sung.com>,
        Dan Williams <dan.j.williams@...el.com>
CC:     "alison.schofield@...el.com" <alison.schofield@...el.com>,
        "vishal.l.verma@...el.com" <vishal.l.verma@...el.com>,
        "ira.weiny@...el.com" <ira.weiny@...el.com>,
        "bwidawsk@...nel.org" <bwidawsk@...nel.org>,
        "Jonathan.Cameron@...wei.com" <Jonathan.Cameron@...wei.com>,
        "linux-cxl@...r.kernel.org" <linux-cxl@...r.kernel.org>,
        Adam Manzanares <a.manzanares@...sung.com>,
        "dave@...olabs.net" <dave@...olabs.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] cxl/hdm: Fix hdm decoder init by adding COMMIT field
 check

Fan Ni wrote:
[..]
> > I think a separate fix for that crash is needed, can you send the
> > backtrace? I.e. I worry that crash can be triggered by other means.
> Hi Dan,
> See backtrace below.

Thanks, I'll take a look. 

[..]
> > > @@ -710,10 +711,11 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
> > >  	base = ioread64_hi_lo(hdm + CXL_HDM_DECODER0_BASE_LOW_OFFSET(which));
> > >  	size = ioread64_hi_lo(hdm + CXL_HDM_DECODER0_SIZE_LOW_OFFSET(which));
> > >  	committed = !!(ctrl & CXL_HDM_DECODER0_CTRL_COMMITTED);
> > > +	should_commit = !!(ctrl & CXL_HDM_DECODER0_CTRL_COMMIT);
> > 
> > This change looks like a good idea in general given the ambiguity of
> > 'committed'. However just combine the two checks into the @committed
> > variable with something like this:
> > 
> > commit_mask = CXL_HDM_DECODER0_CTRL_COMMITTED|CXL_HDM_DECODER0_CTRL_COMMIT;
> > committed = (ctrl & commit_mask) == commit_mask;

Did you also notice this ^^^ request for a fixed up version of the
current patch?