linux-kernel - Re: [PATCH] genirq/affinity: Assign default affinity to pre/post vectors

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:   Fri, 18 Jan 2019 09:52:00 +0800
From:   Huacai Chen <chenhuacai@...il.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     linux-kernel <linux-kernel@...r.kernel.org>,
        Fuxin Zhang <zhangfx@...ote.com>,
        wuzhangjin <wuzhangjin@...il.com>,
        stable <stable@...r.kernel.org>, Christoph Hellwig <hch@....de>,
        Michael Hernandez <michael.hernandez@...ium.com>
Subject: Re: [PATCH] genirq/affinity: Assign default affinity to pre/post vectors

On Thu, Jan 17, 2019 at 9:14 AM 陈华才 <chenhc@...ote.com> wrote:
>
> 陈华才
> 江苏中科梦兰电子科技有限公司/自主安全事业部/软件部
> 江苏常熟虞山镇梦兰村
>
>
>
> ------------------ Original ------------------
> From:  "Thomas Gleixner"<tglx@...utronix.de>;
> Date:  Wed, Jan 16, 2019 05:26 PM
> To:  "陈华才"<chenhc@...ote.com>;
> Cc:  "linux-kernel"<linux-kernel@...r.kernel.org>; "Fuxin Zhang"<zhangfx@...ote.com>; "wuzhangjin"<wuzhangjin@...il.com>; "stable"<stable@...r.kernel.org>; "Christoph Hellwig"<hch@....de>; "Michael Hernandez"<michael.hernandez@...ium.com>;
> Subject:  Re: [PATCH] genirq/affinity: Assign default affinity to pre/post vectors
>
> Chen,
>
> On Wed, 16 Jan 2019, 陈华才 wrote:
>
> > please do not top-post and use line breaks around 78 char.
>
> > I'm not removing all return NULL of irq_create_affinity_masks(), so the ......
>
> > > Moved content to the place where it belongs so the context is preserved.
>
> > ------------------ Original ------------------
> > From:  "Thomas Gleixner"<tglx@...utronix.de>;
> > Date:  Wed, Jan 16, 2019 03:10 AM
> > To:  "Huacai Chen"<chenhc@...ote.com>;
> > Cc:  "linux-kernel"<linux-kernel@...r.kernel.org>; "Fuxin Zhang"<zhangfx@...ote.com>; "wuzhangjin"<wuzhangjin@...il.com>; "stable"<stable@...r.kernel.org>; "Christoph Hellwig"<hch@....de>; "Michael Hernandez"<michael.hernandez@...ium.com>;
> > Subject:  Re: [PATCH] genirq/affinity: Assign default affinity to pre/post vectors
>
> > And please configure your e-mail client to NOT copy the full headers into
> > the reply.
>
> > > > On Mon, 31 Dec 2018, Huacai Chen wrote:
> > > >
> > > > > Generally, irq_create_affinity_masks() assign default affinity to pre/
> > > > > post vectors correctly. However, it ignore the case that there are only
> > > > > pre/post vectors (when nvecs == affd->pre_vectors + affd->post_vectors)
> > > > > and return NULL. This case usually happens when nvecs = 1 (e.g. in nvme
> > > > > driver when MSI-X is unavailable and fallback to MSI) and will trigger
> > > > > the warning in pci_irq_get_affinity(). This patch fix the corner case.
> > > >
> > > > Errm. This is just wrong. When this function returns NULL, then it has
> > > > failed and the caller or any subsequent code is not supposed to use the
> > > > result.
> > > >
> > > > The function can return NULL for other reasons, e.g. when the memory
> > > > allocation failed. How are you going to duct tape that one?
> > >
> > > I'm not removing all return NULL of irq_create_affinity_masks(), so the
> > > memory allocation failure still return NULL. I just handle the case that
> > > there are not enough irq vectors. E.g. in nvme driver, the caller may call
> > > irq_create_affinity_masks() with nvecs=1,pre_vectors=1,post_vectors=0. In
> > > this case, the only one vector's default affinity assigning is skipped.
> >
> > I did not say that you removed all NULL returns. I said that this function
> > can return NULL for other reasons and then the same situation will happen.
> >
> > If the masks pointer returned is NULL then the calling code or any
> > subsequent usage needs to handle it properly. Yes, I understand that this
> > change makes the warning go away for that particular case, but that's not
> > making it any more correct.

Hi, Thomas,

I don't think "nvecs == affd->pre_vectors + affd->post_vectors" is an ERROR,
so it should be different with "return NULL for other reasons" to the caller. If
the caller fallback from MSI-X to MSI, it is probably "nvecs=1,pre_vectors=1,
post_vectors=0". The caller can work perfectly, if pre/post vectors are filled
with the default affinity.

Huacai

> > Thanks,
>
> > tglx