linux-kernel - Re: [PATCH v2 1/2] cxl/cdat: Handle cdat table build errors

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] [day] [month] [year] [list]

Date: Tue, 9 Jan 2024 15:34:58 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Ira Weiny <ira.weiny@...el.com>
CC: fan <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 1/2] cxl/cdat: Handle cdat table build errors

On Mon, 8 Jan 2024 18:48:48 -0800
Ira Weiny <ira.weiny@...el.com> wrote:

> Jonathan Cameron wrote:
> 
> [snip]
> 
> > > > > > 
> > > > > > I did not go that far as I am unsure as well.      
> > > > > Memory allocations in qemu don't fail (well if they do it crashes)
> > > > > Side effect of using glib which makes for simpler cases.
> > > > > https://docs.gtk.org/glib/func.malloc.html
> > > > > 
> > > > > There shouldn't even be any checks :(  I'll fix that up at somepoint
> > > > > across all the CXL emulation.  Sometimes reviewers noticed and
> > > > > we dropped it at earlier stages, but clearly didn't catch them all.
> > > > > 
> > > > > Which come to think of it is why this error condition is in practice
> > > > > not actually buggy as the code won't ever manage to return -ENOMEM and
> > > > > I don't think there are other error codes.      
> > > > 
> > > > Ah.  Ok but in that case I would say that build_cdat_table() should never
> > > > return < 0 to be clear at this level what can happen.
> > > > 
> > > > Would you like a patch for that?  (/me assumes you dropped this patch)    
> > > 
> > > Probably needs to first rip out all the -ENOMEM returns that got into
> > > the CXL code in general, then tidy up the return type to be unsigned.
> > > 
> > > If you want to do that it would be welcome!  
> > Actually.  Build_cdat_table() can return errors just not for this reason.
> > 
> > host_memory_backend_get_memory() can fail for example.  
> 
> I must be on a different version because I don't see that.
> 
> >
> > So original patch is good
> > as is, just that the discussion of memory allocation failure threw me
> > off and should be cleaned up separately.
> >   
> 
> I did this testing on Fan's DCD version...  :-/  ... probably very out of
> date.

https://elixir.bootlin.com/qemu/latest/source/hw/mem/cxl_type3.c#L183
https://elixir.bootlin.com/qemu/v8.1.0/source/hw/mem/cxl_type3.c#L171
been there a while, but meh, too many branches floating around :)

> 
> Fan do you have a newer version than your 2023-11-16 branch?
> 


> Ira
>