linux-kernel - Re: [PATCH v2] ib umem: bugfix: mixed put_pid()s in ib_umem

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, 30 Dec 2016 08:55:10 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Kenneth Lee <liguozhu@...ilicon.com>
Cc:     dledford@...hat.com, sean.hefty@...el.com,
        hal.rosenstock@...il.com, robin.murphy@....com, jroedel@...e.de,
        egtvedt@...fundet.no, vgupta@...opsys.com,
        dave.hansen@...ux.intel.com, lstoakes@...il.com, krzk@...nel.org,
        sebott@...ux.vnet.ibm.com, markb@...lanox.com,
        linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get()

On Fri, Dec 30, 2016 at 12:50:11PM +0800, Kenneth Lee wrote:
> Hi, Leon,
>
> 1. I do change the title except for the version number itself:) But my English
> is quite bad, maybe the title is still quite stupid. I can update it according
> to your advice.

Yes, please
The main points are:
1. Remove "bugifix", it is not needed.
2. Use description in the title and not function names.

>
> 2. I catched the bug by reading the final code, not by bisect-ing the old
> commit. Do you means I should find out which commit introducing the bug? It will
> not be easily to say which it is because it is a "missing bug", rather than a
> "introduced bug". Indicate the commit may not help to remove a patch/commit from
> the stable tree.

The fixes line won't cause for removal of commit, but to addition of
yours on top of their code base.

git blame is your friend.

one fixes line is:
Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging regions")

and the second line is !!!!! NOT !!!!!, you need to go deeper in the logs !!!!!!
Fixes: f7c6a7b5d599 ("IB/uverbs: Export ib_umem_get()/ib_umem_release() to modules")

>
> Could you please give more suggestion? Thanks.

Please, don't use top-posting for this mailing list.
It is really-really annoying.

>
> On Thu, Dec 29, 2016 at 10:17:56AM +0200, Leon Romanovsky wrote:
> > Date: Thu, 29 Dec 2016 10:17:56 +0200
> > From: Leon Romanovsky <leon@...nel.org>
> > To: Kenneth Lee <liguozhu@...ilicon.com>
> > CC: dledford@...hat.com, sean.hefty@...el.com, hal.rosenstock@...il.com,
> >  robin.murphy@....com, jroedel@...e.de, egtvedt@...fundet.no,
> >  vgupta@...opsys.com, dave.hansen@...ux.intel.com, lstoakes@...il.com,
> >  krzk@...nel.org, sebott@...ux.vnet.ibm.com, markb@...lanox.com,
> >  linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org
> > Subject: Re: [PATCH v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get()
> > User-Agent: Mutt/1.7.2 (2016-11-26)
> > Message-ID: <20161229081756.GI26885@...-leonro.local>
> >
> > On Thu, Dec 29, 2016 at 04:27:28PM +0800, Kenneth Lee wrote:
> > > There are two bugfixes in this patch:
> > >
> > > 1. When the execution go to the ib_umem_odp_get() path, pid should be put
> > >    back.
> > > 2. When the memory allocation fail, the pid also should be put back before
> > >    exit.
> > >
> > > Signed-off-by: Kenneth Lee <liguozhu@...ilicon.com>
> > > Reviewed-by: Haggai Eran <haggaie@...lanox.com>
> > > ---
> > > Change from v1 to v2:
> > >   Correcting the patch title and description
> >
> > I don't see any changes except version in the title.
> > What about anything like this?
> > [PATCH v3] IB/umem: Release pid in error and ODP flows
> >
> > And Fixes line please, it will help to forward it to stable trees.
> >
> > Thanks
>
>
>
> --
> 			-Kenneth(Hisilicon)

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)