linux-kernel - RE: [EXTERNAL] [PATCH 1/2] PCI: hv: Use IDR to generate transaction IDs for VMBus hardening

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]

Date:   Sun, 20 Mar 2022 05:53:29 +0000
From:   Saurabh Singh Sengar <ssengar@...rosoft.com>
To:     Andrea Parri <parri.andrea@...il.com>
CC:     KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
        "Michael Kelley (LINUX)" <mikelley@...rosoft.com>,
        Wei Hu <weh@...rosoft.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Wilczynski <kw@...ux.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [EXTERNAL] [PATCH 1/2] PCI: hv: Use IDR to generate transaction
 IDs for VMBus hardening



> -----Original Message-----
> From: Andrea Parri <parri.andrea@...il.com>
> Sent: 19 March 2022 21:29
> To: Saurabh Singh Sengar <ssengar@...rosoft.com>
> Cc: KY Srinivasan <kys@...rosoft.com>; Haiyang Zhang
> <haiyangz@...rosoft.com>; Stephen Hemminger
> <sthemmin@...rosoft.com>; Wei Liu <wei.liu@...nel.org>; Dexuan Cui
> <decui@...rosoft.com>; Michael Kelley (LINUX) <mikelley@...rosoft.com>;
> Wei Hu <weh@...rosoft.com>; Lorenzo Pieralisi
> <lorenzo.pieralisi@....com>; Rob Herring <robh@...nel.org>; Krzysztof
> Wilczynski <kw@...ux.com>; Bjorn Helgaas <bhelgaas@...gle.com>; linux-
> pci@...r.kernel.org; linux-hyperv@...r.kernel.org; linux-
> kernel@...r.kernel.org
> Subject: Re: [EXTERNAL] [PATCH 1/2] PCI: hv: Use IDR to generate transaction
> IDs for VMBus hardening
> 
> > > @@ -1208,6 +1211,27 @@ static void hv_pci_read_config_compl(void
> > > *context, struct pci_response *resp,
> > >  	complete(&comp->comp_pkt.host_event);
> > >  }
> > >
> > > +static inline int alloc_request_id(struct hv_pcibus_device *hbus,
> > > +				   void *ptr, gfp_t gfp)
> > > +{
> > > +	unsigned long flags;
> > > +	int req_id;
> > > +
> > > +	spin_lock_irqsave(&hbus->idr_lock, flags);
> > > +	req_id = idr_alloc(&hbus->idr, ptr, 1, 0, gfp);
> >
> > [Saurabh Singh Sengar] Many a place we are using alloc_request_id with
> GFP_KERNEL, which results this allocation inside of spin lock with
> GFP_KERNEL.
> 
> That's a bug.
> 
> 
> > Is this a good opportunity to use idr_preload ?
> 
> I'd rather fix (and 'simplify' a bit the interface) by doing:
> 
> static inline int alloc_request_id(struct hv_pcibus_device *hbus, void *ptr)
> {
> 	unsigned long flags;
> 	int req_id;
> 
> 	spin_lock_irqsave(&hbus->idr_lock, flags);
> 	req_id = idr_alloc(&hbus->idr, ptr, 1, 0, GFP_ATOMIC);
> 	spin_unlock_irqrestore(&hbus->idr_lock, flags);
> 	return req_id;
> }
> 
> Thoughts?
[Saurabh Sengar] Yes, if we are fine to use GFP_ATOMIC, this makes perfect sense.
Once fixed, please add: Reviewed-by: Saurabh Sengar <ssengar@...rosoft.com>

> 
> Thanks,
>   Andrea