Re: [PATCH 037/190] Revert "RDMA/core: Fix several reference count leaks."

[Jason Gunthorpe]

On Tue, Apr 27, 2021 at 04:27:57PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Apr 21, 2021 at 11:14:44AM -0300, Jason Gunthorpe wrote:
> > On Wed, Apr 21, 2021 at 02:58:32PM +0200, Greg Kroah-Hartman wrote:
> > > This reverts commit 0b8e125e213204508e1b3c4bdfe69713280b7abd.
> > > 
> > > Commits from @umn.edu addresses have been found to be submitted in "bad
> > > faith" to try to test the kernel community's ability to review "known
> > > malicious" changes.  The result of these submissions can be found in a
> > > paper published at the 42nd IEEE Symposium on Security and Privacy
> > > entitled, "Open Source Insecurity: Stealthily Introducing
> > > Vulnerabilities via Hypocrite Commits" written by Qiushi Wu (University
> > > of Minnesota) and Kangjie Lu (University of Minnesota).
> > > 
> > > Because of this, all submissions from this group must be reverted from
> > > the kernel tree and will need to be re-reviewed again to determine if
> > > they actually are a valid fix.  Until that work is complete, remove this
> > > change to ensure that no problems are being introduced into the
> > > codebase.
> > > 
> > > Cc: https
> > > Cc: Qiushi Wu <wu000273@xxxxxxx>
> > > Cc: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> > > Cc: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > >  drivers/infiniband/core/sysfs.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > The original commit is certainly moving things closer to being correct
> > but I'm concerned the entire area is still not bug free, it will take
> > me some time to check the whole code. Reverting seems reasonable for
> > now
> 
> I just re-reviewed it, and it looks like the original commit is ok, so I
> will drop this.

This was what I thought too when I originally applied it..

> But ugh, this is horrid code, you should NOT be dealing with "raw"
> kobjects here at all, just to try to make a subdirectory tree.  That
> causes a total mess as this function shows and probably should be
> cleaned up sometime as obviously userspace tools are not properly seeing
> the objects you are creating here (hint, you can not put a kobject below
> a 'struct device' in the device tree and have udev and others see it
> properly...)

We've talked about this specifically before:

http://lore.kernel.org/r/20210331170720.GY2710221@xxxxxxxx

I still don't understand what you mean by "udev sees it properly", as
above, all the tests I thought of look OK.

It is uABI at this point that real userspace software relies on, so we
can't change the sysfs paths. If there is a more right way to code it
then let me know I can have a go - Kees also recently found some bad
stuff here I hope to get to soon.

Jason