| Message ID | 20210407001658.2208535-1-pakki001@umn.edu |
|---|---|
| State | New |
| Headers | show |
| Series | SUNRPC: Add a check for gss_release_msg | expand |
On Thu, Apr 8, 2021 at 11:01 AM Trond Myklebust <trondmy@hammerspace.com> wrote: > > On Tue, 2021-04-06 at 19:16 -0500, Aditya Pakki wrote: > > In gss_pipe_destroy_msg(), in case of error in msg, gss_release_msg > > deletes gss_msg. The patch adds a check to avoid a potential double > > free. > > > > Signed-off-by: Aditya Pakki <pakki001@umn.edu> > > --- > > net/sunrpc/auth_gss/auth_gss.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/net/sunrpc/auth_gss/auth_gss.c > > b/net/sunrpc/auth_gss/auth_gss.c > > index 5f42aa5fc612..eb52eebb3923 100644 > > --- a/net/sunrpc/auth_gss/auth_gss.c > > +++ b/net/sunrpc/auth_gss/auth_gss.c > > @@ -848,7 +848,8 @@ gss_pipe_destroy_msg(struct rpc_pipe_msg *msg) > > warn_gssd(); > > gss_release_msg(gss_msg); > > } > > - gss_release_msg(gss_msg); > > + if (gss_msg) > > + gss_release_msg(gss_msg); > > } > > > > static void gss_pipe_dentry_destroy(struct dentry *dir, > > > NACK. There's no double free there. I disagree that there is no double free, the wording of the commit describes the problem in the error case is that we call gss_release_msg() and then we call it again but the 1st one released the gss_msg. However, I think the fix should probably be instead: diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c index 5f42aa5fc612..e8aae617e981 100644 --- a/net/sunrpc/auth_gss/auth_gss.c +++ b/net/sunrpc/auth_gss/auth_gss.c @@ -846,7 +846,7 @@ gss_pipe_destroy_msg(struct rpc_pipe_msg *msg) gss_unhash_msg(gss_msg); if (msg->errno == -ETIMEDOUT) warn_gssd(); - gss_release_msg(gss_msg); + return gss_release_msg(gss_msg); } gss_release_msg(gss_msg); } > > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com > >
On Thu, 2021-04-08 at 11:24 -0400, Olga Kornievskaia wrote: > On Thu, Apr 8, 2021 at 11:01 AM Trond Myklebust < > trondmy@hammerspace.com> wrote: > > > > On Tue, 2021-04-06 at 19:16 -0500, Aditya Pakki wrote: > > > In gss_pipe_destroy_msg(), in case of error in msg, > > > gss_release_msg > > > deletes gss_msg. The patch adds a check to avoid a potential > > > double > > > free. > > > > > > Signed-off-by: Aditya Pakki <pakki001@umn.edu> > > > --- > > > net/sunrpc/auth_gss/auth_gss.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/net/sunrpc/auth_gss/auth_gss.c > > > b/net/sunrpc/auth_gss/auth_gss.c > > > index 5f42aa5fc612..eb52eebb3923 100644 > > > --- a/net/sunrpc/auth_gss/auth_gss.c > > > +++ b/net/sunrpc/auth_gss/auth_gss.c > > > @@ -848,7 +848,8 @@ gss_pipe_destroy_msg(struct rpc_pipe_msg > > > *msg) > > > warn_gssd(); > > > gss_release_msg(gss_msg); > > > } > > > - gss_release_msg(gss_msg); > > > + if (gss_msg) > > > + gss_release_msg(gss_msg); > > > } > > > > > > static void gss_pipe_dentry_destroy(struct dentry *dir, > > > > > > NACK. There's no double free there. > > I disagree that there is no double free, the wording of the commit > describes the problem in the error case is that we call > gss_release_msg() and then we call it again but the 1st one released > the gss_msg. However, I think the fix should probably be instead: > diff --git a/net/sunrpc/auth_gss/auth_gss.c > b/net/sunrpc/auth_gss/auth_gss.c > index 5f42aa5fc612..e8aae617e981 100644 > --- a/net/sunrpc/auth_gss/auth_gss.c > +++ b/net/sunrpc/auth_gss/auth_gss.c > @@ -846,7 +846,7 @@ gss_pipe_destroy_msg(struct rpc_pipe_msg *msg) > gss_unhash_msg(gss_msg); > if (msg->errno == -ETIMEDOUT) > warn_gssd(); > - gss_release_msg(gss_msg); > + return gss_release_msg(gss_msg); > } > gss_release_msg(gss_msg); > } > Please look one line further up: there is a refcount_inc() that matches that first gss_release_msg(). Removing that call results in a reintroduction of the leak that Neil fixed in commit 1cded9d2974fe ("SUNRPC: fix refcounting problems with auth_gss messages."). We might, however, instead opt to remove both the refcount_inc() and matching gss_release_msg(). Those do seem to be redundant, given that the caller also holds a refcount. > > > > -- > > Trond Myklebust > > Linux NFS client maintainer, Hammerspace > > trond.myklebust@hammerspace.com > > > > -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@hammerspace.com
On Tue, Apr 06, 2021 at 07:16:56PM -0500, Aditya Pakki wrote: > In gss_pipe_destroy_msg(), in case of error in msg, gss_release_msg > deletes gss_msg. The patch adds a check to avoid a potential double > free. > > Signed-off-by: Aditya Pakki <pakki001@umn.edu> > --- > net/sunrpc/auth_gss/auth_gss.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c > index 5f42aa5fc612..eb52eebb3923 100644 > --- a/net/sunrpc/auth_gss/auth_gss.c > +++ b/net/sunrpc/auth_gss/auth_gss.c > @@ -848,7 +848,8 @@ gss_pipe_destroy_msg(struct rpc_pipe_msg *msg) > warn_gssd(); > gss_release_msg(gss_msg); > } > - gss_release_msg(gss_msg); > + if (gss_msg) > + gss_release_msg(gss_msg);a If you look at the code, this is impossible to have happen. Please stop submitting known-invalid patches. Your professor is playing around with the review process in order to achieve a paper in some strange and bizarre way. This is not ok, it is wasting our time, and we will have to report this, AGAIN, to your university... greg k-h
On Tue, Apr 20, 2021 at 09:15:23AM +0200, Greg KH wrote: > If you look at the code, this is impossible to have happen. > > Please stop submitting known-invalid patches. Your professor is playing > around with the review process in order to achieve a paper in some > strange and bizarre way. > > This is not ok, it is wasting our time, and we will have to report this, > AGAIN, to your university... What's the story here? --b.
On Tue, Apr 20, 2021 at 01:10:08PM -0400, J. Bruce Fields wrote: > On Tue, Apr 20, 2021 at 09:15:23AM +0200, Greg KH wrote: > > If you look at the code, this is impossible to have happen. > > > > Please stop submitting known-invalid patches. Your professor is playing > > around with the review process in order to achieve a paper in some > > strange and bizarre way. > > > > This is not ok, it is wasting our time, and we will have to report this, > > AGAIN, to your university... > > What's the story here? Those commits are part of the following research: https://github.com/QiushiWu/QiushiWu.github.io/blob/main/papers/OpenSourceInsecurity.pdf They introduce kernel bugs on purpose. Yesterday, I took a look on 4 accepted patches from Aditya and 3 of them added various severity security "holes". Thanks > > --b.
On Wed, Apr 21, 2021 at 08:10:25AM +0300, Leon Romanovsky wrote: > On Tue, Apr 20, 2021 at 01:10:08PM -0400, J. Bruce Fields wrote: > > On Tue, Apr 20, 2021 at 09:15:23AM +0200, Greg KH wrote: > > > If you look at the code, this is impossible to have happen. > > > > > > Please stop submitting known-invalid patches. Your professor is playing > > > around with the review process in order to achieve a paper in some > > > strange and bizarre way. > > > > > > This is not ok, it is wasting our time, and we will have to report this, > > > AGAIN, to your university... > > > > What's the story here? > > Those commits are part of the following research: > https://github.com/QiushiWu/QiushiWu.github.io/blob/main/papers/OpenSourceInsecurity.pdf > > They introduce kernel bugs on purpose. Yesterday, I took a look on 4 > accepted patches from Aditya and 3 of them added various severity security > "holes". All contributions by this group of people need to be reverted, if they have not been done so already, as what they are doing is intentional malicious behavior and is not acceptable and totally unethical. I'll look at it after lunch unless someone else wants to do it... greg k-h
On Wed, Apr 21, 2021 at 07:43:03AM +0200, Greg KH wrote: > On Wed, Apr 21, 2021 at 08:10:25AM +0300, Leon Romanovsky wrote: > > On Tue, Apr 20, 2021 at 01:10:08PM -0400, J. Bruce Fields wrote: > > > On Tue, Apr 20, 2021 at 09:15:23AM +0200, Greg KH wrote: > > > > If you look at the code, this is impossible to have happen. > > > > > > > > Please stop submitting known-invalid patches. Your professor is playing > > > > around with the review process in order to achieve a paper in some > > > > strange and bizarre way. > > > > > > > > This is not ok, it is wasting our time, and we will have to report this, > > > > AGAIN, to your university... > > > > > > What's the story here? > > > > Those commits are part of the following research: > > https://github.com/QiushiWu/QiushiWu.github.io/blob/main/papers/OpenSourceInsecurity.pdf > > > > They introduce kernel bugs on purpose. Yesterday, I took a look on 4 > > accepted patches from Aditya and 3 of them added various severity security > > "holes". > > All contributions by this group of people need to be reverted, if they > have not been done so already, as what they are doing is intentional > malicious behavior and is not acceptable and totally unethical. I'll > look at it after lunch unless someone else wants to do it... It looks like the best possible scenario. I asked to revert the bad patch, but didn't get any response yet. https://lore.kernel.org/lkml/YH6aMsbqruMZiWFe@unreal/ > > greg k-h
A: http://en.wikipedia.org/wiki/Top_post Q: Were do I find info about this thing called top-posting? A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? A: No. Q: Should I include quotations after my reply? http://daringfireball.net/2007/07/on_top On Wed, Apr 21, 2021 at 02:56:27AM -0500, Aditya Pakki wrote: > Greg, > > I respectfully ask you to cease and desist from making wild accusations > that are bordering on slander. > > These patches were sent as part of a new static analyzer that I wrote and > it's sensitivity is obviously not great. I sent patches on the hopes to get > feedback. We are not experts in the linux kernel and repeatedly making > these statements is disgusting to hear. > > Obviously, it is a wrong step but your preconceived biases are so strong > that you make allegations without merit nor give us any benefit of doubt. > > I will not be sending any more patches due to the attitude that is not only > unwelcome but also intimidating to newbies and non experts. You, and your group, have publicly admitted to sending known-buggy patches to see how the kernel community would react to them, and published a paper based on that work. Now you submit a new series of obviously-incorrect patches again, so what am I supposed to think of such a thing? They obviously were _NOT_ created by a static analysis tool that is of any intelligence, as they all are the result of totally different patterns, and all of which are obviously not even fixing anything at all. So what am I supposed to think here, other than that you and your group are continuing to experiment on the kernel community developers by sending such nonsense patches? When submitting patches created by a tool, everyone who does so submits them with wording like "found by tool XXX, we are not sure if this is correct or not, please advise." which is NOT what you did here at all. You were not asking for help, you were claiming that these were legitimate fixes, which you KNEW to be incorrect. A few minutes with anyone with the semblance of knowledge of C can see that your submissions do NOT do anything at all, so to think that a tool created them, and then that you thought they were a valid "fix" is totally negligent on your part, not ours. You are the one at fault, it is not our job to be the test subjects of a tool you create. Our community welcomes developers who wish to help and enhance Linux. That is NOT what you are attempting to do here, so please do not try to frame it that way. Our community does not appreciate being experimented on, and being "tested" by submitting known patches that are either do nothing on purpose, or introduce bugs on purpose. If you wish to do work like this, I suggest you find a different community to run your experiments on, you are not welcome here. Because of this, I will now have to ban all future contributions from your University and rip out your previous contributions, as they were obviously submitted in bad-faith with the intent to cause problems. *plonk* greg k-h
Hi Greg, On Wed, Apr 21, 2021 at 6:44 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Wed, Apr 21, 2021 at 08:10:25AM +0300, Leon Romanovsky wrote: > > On Tue, Apr 20, 2021 at 01:10:08PM -0400, J. Bruce Fields wrote: > > > On Tue, Apr 20, 2021 at 09:15:23AM +0200, Greg KH wrote: > > > > If you look at the code, this is impossible to have happen. > > > > <snip> > > They introduce kernel bugs on purpose. Yesterday, I took a look on 4 > > accepted patches from Aditya and 3 of them added various severity security > > "holes". > > All contributions by this group of people need to be reverted, if they > have not been done so already, as what they are doing is intentional > malicious behavior and is not acceptable and totally unethical. I'll > look at it after lunch unless someone else wants to do it... A lot of these have already reached the stable trees. I can send you revert patches for stable by the end of today (if your scripts have not already done it). -- Regards Sudip
On Wed, Apr 21, 2021 at 11:07:11AM +0100, Sudip Mukherjee wrote: > Hi Greg, > > On Wed, Apr 21, 2021 at 6:44 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Wed, Apr 21, 2021 at 08:10:25AM +0300, Leon Romanovsky wrote: > > > On Tue, Apr 20, 2021 at 01:10:08PM -0400, J. Bruce Fields wrote: > > > > On Tue, Apr 20, 2021 at 09:15:23AM +0200, Greg KH wrote: > > > > > If you look at the code, this is impossible to have happen. > > > > > > > <snip> > > > > They introduce kernel bugs on purpose. Yesterday, I took a look on 4 > > > accepted patches from Aditya and 3 of them added various severity security > > > "holes". > > > > All contributions by this group of people need to be reverted, if they > > have not been done so already, as what they are doing is intentional > > malicious behavior and is not acceptable and totally unethical. I'll > > look at it after lunch unless someone else wants to do it... > > A lot of these have already reached the stable trees. I can send you > revert patches for stable by the end of today (if your scripts have > not already done it). Yes, if you have a list of these that are already in the stable trees, that would be great to have revert patches, it would save me the extra effort these mess is causing us to have to do... thanks, greg k-h
>> >>>> They introduce kernel bugs on purpose. Yesterday, I took a look on 4 >>>> accepted patches from Aditya and 3 of them added various severity security >>>> "holes". >>> >>> All contributions by this group of people need to be reverted, if they >>> have not been done so already, as what they are doing is intentional >>> malicious behavior and is not acceptable and totally unethical. I'll >>> look at it after lunch unless someone else wants to do it… >> <snip> Academic research should NOT waste the time of a community. If you believe this behavior deserves an escalation, you can contact the Institutional Review Board (irb@umn.edu) at UMN to investigate whether this behavior was harmful; in particular, whether the research activity had an appropriate IRB review, and what safeguards prevent repeats in other communities. All researchers at UMN must comply with their Human Research Protection Program Plan [1], and the UMN worksheet [2] to determine if a research activity needs IRB approval includes this question: ==== Will the investigator use, study, or analyze information or biospecimens obtained through either of the following mechanisms,? Specify which mechanism(s) apply, if yes: … Communication or interpersonal contact with the individuals. ("interaction”). === which I believe is true based on this thread. [1] Human Research Protection Program Plan https://drive.google.com/file/d/0B7644h9N2vLcV3FyMzJKYnJGeDA/view [2] Human Research Determination https://drive.google.com/file/d/0Bw4LRE9kGb69Mm5TbldxSVkwTms/view
On Wed, Apr 21, 2021 at 11:58:08AM +0000, Shelat, Abhi wrote: > >> > >>>> They introduce kernel bugs on purpose. Yesterday, I took a look on 4 > >>>> accepted patches from Aditya and 3 of them added various severity security > >>>> "holes". > >>> > >>> All contributions by this group of people need to be reverted, if they > >>> have not been done so already, as what they are doing is intentional > >>> malicious behavior and is not acceptable and totally unethical. I'll > >>> look at it after lunch unless someone else wants to do it… > >> > > <snip> > > Academic research should NOT waste the time of a community. Thank you for saying this, I appreciate it. greg k-h
On Wed, Apr 21, 2021 at 11:58:08AM +0000, Shelat, Abhi wrote: > >> > >>>> They introduce kernel bugs on purpose. Yesterday, I took a look on 4 > >>>> accepted patches from Aditya and 3 of them added various severity security > >>>> "holes". > >>> > >>> All contributions by this group of people need to be reverted, if they > >>> have not been done so already, as what they are doing is intentional > >>> malicious behavior and is not acceptable and totally unethical. I'll > >>> look at it after lunch unless someone else wants to do it… > >> > > <snip> > > Academic research should NOT waste the time of a community. > > If you believe this behavior deserves an escalation, you can contact the Institutional Review Board (irb@umn.edu) at UMN to investigate whether this behavior was harmful; in particular, whether the research activity had an appropriate IRB review, and what safeguards prevent repeats in other communities. The huge advantage of being "community" is that we don't need to do all the above and waste our time to fill some bureaucratic forms with unclear timelines and results. Our solution to ignore all @umn.edu contributions is much more reliable to us who are suffering from these researchers. Thanks
On Wed, Apr 21, 2021 at 2:07 AM Leon Romanovsky <leon@kernel.org> wrote: > > On Tue, Apr 20, 2021 at 01:10:08PM -0400, J. Bruce Fields wrote: > > On Tue, Apr 20, 2021 at 09:15:23AM +0200, Greg KH wrote: > > > If you look at the code, this is impossible to have happen. > > > > > > Please stop submitting known-invalid patches. Your professor is playing > > > around with the review process in order to achieve a paper in some > > > strange and bizarre way. > > > > > > This is not ok, it is wasting our time, and we will have to report this, > > > AGAIN, to your university... > > > > What's the story here? > > Those commits are part of the following research: > https://github.com/QiushiWu/QiushiWu.github.io/blob/main/papers/OpenSourceInsecurity.pdf This thread is the first I'm hearing about this. I wonder if there is a good way of alerting the entire kernel community (including those only subscribed to subsystem mailing lists) about what's going on? It seems like useful information to have to push back against these patches. Anna > > They introduce kernel bugs on purpose. Yesterday, I took a look on 4 > accepted patches from Aditya and 3 of them added various severity security > "holes". > > Thanks > > > > > --b.
On Wed, 2021-04-21 at 15:19 +0300, Leon Romanovsky wrote: > On Wed, Apr 21, 2021 at 11:58:08AM +0000, Shelat, Abhi wrote: > > > > > > > > > > They introduce kernel bugs on purpose. Yesterday, I took a > > > > > > look on 4 > > > > > > accepted patches from Aditya and 3 of them added various > > > > > > severity security > > > > > > "holes". > > > > > > > > > > All contributions by this group of people need to be > > > > > reverted, if they > > > > > have not been done so already, as what they are doing is > > > > > intentional > > > > > malicious behavior and is not acceptable and totally > > > > > unethical. I'll > > > > > look at it after lunch unless someone else wants to do it… > > > > > > > > <snip> > > > > Academic research should NOT waste the time of a community. > > > > If you believe this behavior deserves an escalation, you can > > contact the Institutional Review Board (irb@umn.edu) at UMN to > > investigate whether this behavior was harmful; in particular, > > whether the research activity had an appropriate IRB review, and > > what safeguards prevent repeats in other communities. > > The huge advantage of being "community" is that we don't need to do > all > the above and waste our time to fill some bureaucratic forms with > unclear > timelines and results. > > Our solution to ignore all @umn.edu contributions is much more > reliable > to us who are suffering from these researchers. > <shrug>That's an easy thing to sidestep by just shifting to using a private email address.</shrug> There really is no alternative for maintainers other than to always be sceptical of patches submitted by people who are not known and trusted members of the community, and to scrutinise those patches with more care. -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@hammerspace.com
On Wed, Apr 21, 2021 at 01:11:03PM +0000, Trond Myklebust wrote: > On Wed, 2021-04-21 at 15:19 +0300, Leon Romanovsky wrote: > > On Wed, Apr 21, 2021 at 11:58:08AM +0000, Shelat, Abhi wrote: > > > > > > > > > > > > They introduce kernel bugs on purpose. Yesterday, I took a > > > > > > > look on 4 > > > > > > > accepted patches from Aditya and 3 of them added various > > > > > > > severity security > > > > > > > "holes". > > > > > > > > > > > > All contributions by this group of people need to be > > > > > > reverted, if they > > > > > > have not been done so already, as what they are doing is > > > > > > intentional > > > > > > malicious behavior and is not acceptable and totally > > > > > > unethical. I'll > > > > > > look at it after lunch unless someone else wants to do it… > > > > > > > > > > > <snip> > > > > > > Academic research should NOT waste the time of a community. > > > > > > If you believe this behavior deserves an escalation, you can > > > contact the Institutional Review Board (irb@umn.edu) at UMN to > > > investigate whether this behavior was harmful; in particular, > > > whether the research activity had an appropriate IRB review, and > > > what safeguards prevent repeats in other communities. > > > > The huge advantage of being "community" is that we don't need to do > > all > > the above and waste our time to fill some bureaucratic forms with > > unclear > > timelines and results. > > > > Our solution to ignore all @umn.edu contributions is much more > > reliable > > to us who are suffering from these researchers. > > > > <shrug>That's an easy thing to sidestep by just shifting to using a > private email address.</shrug> > > There really is no alternative for maintainers other than to always be > sceptical of patches submitted by people who are not known and trusted > members of the community, and to scrutinise those patches with more > care. Right, my guess is that many maintainers failed in the trap when they saw respectful address @umn.edu together with commit message saying about "new static analyzer tool". The mental bias here is to say that "oh, another academic group tries to reinvent the wheel, looks ok". Thanks > > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com > >
On Wed, Apr 21, 2021 at 01:11:03PM +0000, Trond Myklebust wrote: > On Wed, 2021-04-21 at 15:19 +0300, Leon Romanovsky wrote: > > On Wed, Apr 21, 2021 at 11:58:08AM +0000, Shelat, Abhi wrote: > > > > > > > > > > > > They introduce kernel bugs on purpose. Yesterday, I took a > > > > > > > look on 4 > > > > > > > accepted patches from Aditya and 3 of them added various > > > > > > > severity security > > > > > > > "holes". > > > > > > > > > > > > All contributions by this group of people need to be > > > > > > reverted, if they > > > > > > have not been done so already, as what they are doing is > > > > > > intentional > > > > > > malicious behavior and is not acceptable and totally > > > > > > unethical. I'll > > > > > > look at it after lunch unless someone else wants to do it… > > > > > > > > > > > <snip> > > > > > > Academic research should NOT waste the time of a community. > > > > > > If you believe this behavior deserves an escalation, you can > > > contact the Institutional Review Board (irb@umn.edu) at UMN to > > > investigate whether this behavior was harmful; in particular, > > > whether the research activity had an appropriate IRB review, and > > > what safeguards prevent repeats in other communities. > > > > The huge advantage of being "community" is that we don't need to do > > all > > the above and waste our time to fill some bureaucratic forms with > > unclear > > timelines and results. > > > > Our solution to ignore all @umn.edu contributions is much more > > reliable > > to us who are suffering from these researchers. > > > > <shrug>That's an easy thing to sidestep by just shifting to using a > private email address.</shrug> If they just want to be jerks, yes. But they can't then use that type of "hiding" to get away with claiming it was done for a University research project as that's even more unethical than what they are doing now. > There really is no alternative for maintainers other than to always be > sceptical of patches submitted by people who are not known and trusted > members of the community, and to scrutinise those patches with more > care. Agreed, and when we notice things like this that were determined to be bad, we have the ability to easily go back and rip the changes out and we can slowly add them back if they are actually something we want to do. Which is what I just did: https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/ thanks, greg k-h
On Wed, Apr 21, 2021 at 03:21:15PM +0200, gregkh@linuxfoundation.org wrote: > On Wed, Apr 21, 2021 at 01:11:03PM +0000, Trond Myklebust wrote: > > On Wed, 2021-04-21 at 15:19 +0300, Leon Romanovsky wrote: > > > On Wed, Apr 21, 2021 at 11:58:08AM +0000, Shelat, Abhi wrote: > > > > > > > > > > > > > > They introduce kernel bugs on purpose. Yesterday, I took a > > > > > > > > look on 4 > > > > > > > > accepted patches from Aditya and 3 of them added various > > > > > > > > severity security > > > > > > > > "holes". > > > > > > > > > > > > > > All contributions by this group of people need to be > > > > > > > reverted, if they > > > > > > > have not been done so already, as what they are doing is > > > > > > > intentional > > > > > > > malicious behavior and is not acceptable and totally > > > > > > > unethical. I'll > > > > > > > look at it after lunch unless someone else wants to do it… > > > > > > > > > > > > > > <snip> > > > > > > > > Academic research should NOT waste the time of a community. > > > > > > > > If you believe this behavior deserves an escalation, you can > > > > contact the Institutional Review Board (irb@umn.edu) at UMN to > > > > investigate whether this behavior was harmful; in particular, > > > > whether the research activity had an appropriate IRB review, and > > > > what safeguards prevent repeats in other communities. > > > > > > The huge advantage of being "community" is that we don't need to do > > > all > > > the above and waste our time to fill some bureaucratic forms with > > > unclear > > > timelines and results. > > > > > > Our solution to ignore all @umn.edu contributions is much more > > > reliable > > > to us who are suffering from these researchers. > > > > > > > <shrug>That's an easy thing to sidestep by just shifting to using a > > private email address.</shrug> > > If they just want to be jerks, yes. But they can't then use that type > of "hiding" to get away with claiming it was done for a University > research project as that's even more unethical than what they are doing > now. > > > There really is no alternative for maintainers other than to always be > > sceptical of patches submitted by people who are not known and trusted > > members of the community, and to scrutinise those patches with more > > care. > > Agreed, and when we notice things like this that were determined to be > bad, we have the ability to easily go back and rip the changes out and > we can slowly add them back if they are actually something we want to > do. > > Which is what I just did: > https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/ Greg, Did you push your series to the public git? I would like to add you a couple of reverts. And do you have a list of not reverted commits? It will save us from doing same comparison of reverted/not reverted over and over. Thanks > > thanks, > > greg k-h
On Wed, Apr 21, 2021 at 11:58:08AM +0000, Shelat, Abhi wrote: > Academic research should NOT waste the time of a community. > > If you believe this behavior deserves an escalation, you can contact > the Institutional Review Board (irb@umn.edu) at UMN to investigate > whether this behavior was harmful; in particular, whether the research > activity had an appropriate IRB review, and what safeguards prevent > repeats in other communities. For what it's worth, they do address security, IRB, and maintainer-time questions in "Ethical Considerations", starting on p. 8: https://github.com/QiushiWu/QiushiWu.github.io/blob/main/papers/OpenSourceInsecurity.pdf (Summary: in that experiment, they claim actual fixes were sent before the original (incorrect) patches had a chance to be committed; that their IRB reviewed the plan and determined it was not human research; and that patches were all small and (after correction) fixed real (if minor) bugs.) This effort doesn't appear to be following similar protocols, if Leon Romanvosky and Aditya Pakki are correct that security holes have already reached stable. Also, I still don't understand the explanation of the original SUNRPC patch. I don't know much about static analyzers, but it really doesn't look like the kind of mistake I'd expect one to make. --b.
On Wed, 21 Apr 2021 16:20:46 +0300 Leon Romanovsky <leon@kernel.org> wrote: > > There really is no alternative for maintainers other than to always be > > sceptical of patches submitted by people who are not known and trusted > > members of the community, and to scrutinise those patches with more > > care. > There's only a couple of contributors to my code that I will take without looking deeply at what it does. And those are well respected developers that many other people know. > Right, my guess is that many maintainers failed in the trap when they > saw respectful address @umn.edu together with commit message saying > about "new static analyzer tool". > > The mental bias here is to say that "oh, another academic group tries > to reinvent the wheel, looks ok". I'm skeptical of all static analyzers, as I've seen too many good ones still produce crappy fixes. I look even more carefully if I see that it was a tool that discovered the bug and not a human. The one patch from Greg's reverts that affects my code was actually a legitimate fix, and looking back at the thread of the submission, I even asked if it was found via inspection or a tool. https://lore.kernel.org/lkml/20190419223718.17fa8246@oasis.local.home/ -- Steve
On Wed, Apr 21, 2021 at 09:37:27AM -0400, J. Bruce Fields wrote: > On Wed, Apr 21, 2021 at 11:58:08AM +0000, Shelat, Abhi wrote: > > Academic research should NOT waste the time of a community. > > > > If you believe this behavior deserves an escalation, you can contact > > the Institutional Review Board (irb@umn.edu) at UMN to investigate > > whether this behavior was harmful; in particular, whether the research > > activity had an appropriate IRB review, and what safeguards prevent > > repeats in other communities. > > For what it's worth, they do address security, IRB, and maintainer-time > questions in "Ethical Considerations", starting on p. 8: > > https://github.com/QiushiWu/QiushiWu.github.io/blob/main/papers/OpenSourceInsecurity.pdf > > (Summary: in that experiment, they claim actual fixes were sent before > the original (incorrect) patches had a chance to be committed; that > their IRB reviewed the plan and determined it was not human research; > and that patches were all small and (after correction) fixed real (if > minor) bugs.) > > This effort doesn't appear to be following similar protocols, if Leon > Romanvosky and Aditya Pakki are correct that security holes have already > reached stable. Aditya Pakki is the one who is sending those patches. If you want to see another accepted patch that is already part of stable@, you are invited to take a look on this patch that has "built-in bug": 8e949363f017 ("net: mlx5: Add a missing check on idr_find, free buf") Thanks
On Wed, Apr 21, 2021 at 04:34:00PM +0300, Leon Romanovsky wrote: > On Wed, Apr 21, 2021 at 03:21:15PM +0200, gregkh@linuxfoundation.org wrote: > > On Wed, Apr 21, 2021 at 01:11:03PM +0000, Trond Myklebust wrote: > > > On Wed, 2021-04-21 at 15:19 +0300, Leon Romanovsky wrote: > > > > On Wed, Apr 21, 2021 at 11:58:08AM +0000, Shelat, Abhi wrote: > > > > > > > > > > > > > > > > They introduce kernel bugs on purpose. Yesterday, I took a > > > > > > > > > look on 4 > > > > > > > > > accepted patches from Aditya and 3 of them added various > > > > > > > > > severity security > > > > > > > > > "holes". > > > > > > > > > > > > > > > > All contributions by this group of people need to be > > > > > > > > reverted, if they > > > > > > > > have not been done so already, as what they are doing is > > > > > > > > intentional > > > > > > > > malicious behavior and is not acceptable and totally > > > > > > > > unethical. I'll > > > > > > > > look at it after lunch unless someone else wants to do it… > > > > > > > > > > > > > > > > > <snip> > > > > > > > > > > Academic research should NOT waste the time of a community. > > > > > > > > > > If you believe this behavior deserves an escalation, you can > > > > > contact the Institutional Review Board (irb@umn.edu) at UMN to > > > > > investigate whether this behavior was harmful; in particular, > > > > > whether the research activity had an appropriate IRB review, and > > > > > what safeguards prevent repeats in other communities. > > > > > > > > The huge advantage of being "community" is that we don't need to do > > > > all > > > > the above and waste our time to fill some bureaucratic forms with > > > > unclear > > > > timelines and results. > > > > > > > > Our solution to ignore all @umn.edu contributions is much more > > > > reliable > > > > to us who are suffering from these researchers. > > > > > > > > > > <shrug>That's an easy thing to sidestep by just shifting to using a > > > private email address.</shrug> > > > > If they just want to be jerks, yes. But they can't then use that type > > of "hiding" to get away with claiming it was done for a University > > research project as that's even more unethical than what they are doing > > now. > > > > > There really is no alternative for maintainers other than to always be > > > sceptical of patches submitted by people who are not known and trusted > > > members of the community, and to scrutinise those patches with more > > > care. > > > > Agreed, and when we notice things like this that were determined to be > > bad, we have the ability to easily go back and rip the changes out and > > we can slowly add them back if they are actually something we want to > > do. > > > > Which is what I just did: > > https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/ > > Greg, > > Did you push your series to the public git? I would like to add you a > couple of reverts. Yes, it can be found here: git@gitolite.kernel.org:/pub/scm/linux/kernel/git/gregkh/driver-core.git umn.edu-reverts You can send reverts in email if you want, whatever works best. > And do you have a list of not reverted commits? It will save us from > doing same comparison of reverted/not reverted over and over. Below is the list that didn't do a simple "revert" that I need to look at. I was going to have my interns look into this, there's no need to bother busy maintainers with it unless you really want to, as I can't tell anyone what to work on :) thanks, greg k-h ------------------------- # commits that need to be looked at as a clean revert did not work 990a1162986e 58d0c864e1a7 a068aab42258 8816cd726a4f c705f9fc6a17 8b6fc114beeb 169f9acae086 8da96730331d f4f5748bfec9 e08f0761234d cb5173594d50 06d5d6b7f994 d9350f21e5fe 6f0ce4dfc5a3 f0d14edd2ba4 46953f97224d 3c77ff8f8bae 0aab8e4df470 8e949363f017 f8ee34c3e77a fd21b79e541e 766460852cfa 41f00e6e9e55 78540a259b05 208c6e8cff1b 7ecced0934e5 48f40b96de2c 9aabb68568b4 2cc12751cf46 534c89c22e26 6a8ca24590a2 d70d70aec963 d7737d425745 3a10e3dd52e8 d6cb77228e3a 517ccc2aa50d 07660ca679da 0fff9bd47e13 6ade657d6125 2795e8c25161 4ec850e5dfec 035a14e71f27 10010493c126 4280b73092fe 5910fa0d0d98 40619f7dd3ef 0a54ea9f481f 44fabd8cdaaa 02cc53e223d4 c99776cc4018 7fc93f3285b1 6ae16dfb61bc 9c6260de505b eb8950861c1b 46273cf7e009 89dfd0083751 c9c63915519b cd07e3701fa6 15b3048aeed8 7172122be6a4 47db7873136a 58f5bbe331c5 6b995f4eec34 8af03d1ae2e1 f16b613ca8b3 6009d1fe6ba3 8e03477cb709 dc487321b1e6
On Wed, Apr 21, 2021 at 04:49:31PM +0300, Leon Romanovsky wrote: > On Wed, Apr 21, 2021 at 09:37:27AM -0400, J. Bruce Fields wrote: > > On Wed, Apr 21, 2021 at 11:58:08AM +0000, Shelat, Abhi wrote: > > > Academic research should NOT waste the time of a community. > > > > > > If you believe this behavior deserves an escalation, you can contact > > > the Institutional Review Board (irb@umn.edu) at UMN to investigate > > > whether this behavior was harmful; in particular, whether the research > > > activity had an appropriate IRB review, and what safeguards prevent > > > repeats in other communities. > > > > For what it's worth, they do address security, IRB, and maintainer-time > > questions in "Ethical Considerations", starting on p. 8: > > > > https://github.com/QiushiWu/QiushiWu.github.io/blob/main/papers/OpenSourceInsecurity.pdf > > > > (Summary: in that experiment, they claim actual fixes were sent before > > the original (incorrect) patches had a chance to be committed; that > > their IRB reviewed the plan and determined it was not human research; > > and that patches were all small and (after correction) fixed real (if > > minor) bugs.) > > > > This effort doesn't appear to be following similar protocols, if Leon > > Romanvosky and Aditya Pakki are correct that security holes have already > > reached stable. > > Aditya Pakki is the one who is sending those patches. Argh, sorry, I I meant Sudip Mukherjee, who reported their reaching stable. Apologies. > If you want to see another accepted patch that is already part of > stable@, you are invited to take a look on this patch that has "built-in bug": > 8e949363f017 ("net: mlx5: Add a missing check on idr_find, free buf") Interesting, thanks. --b.
On Wed, Apr 21, 2021 at 03:50:58PM +0200, gregkh@linuxfoundation.org wrote: > On Wed, Apr 21, 2021 at 04:34:00PM +0300, Leon Romanovsky wrote: > > On Wed, Apr 21, 2021 at 03:21:15PM +0200, gregkh@linuxfoundation.org wrote: > > > On Wed, Apr 21, 2021 at 01:11:03PM +0000, Trond Myklebust wrote: > > > > On Wed, 2021-04-21 at 15:19 +0300, Leon Romanovsky wrote: > > > > > On Wed, Apr 21, 2021 at 11:58:08AM +0000, Shelat, Abhi wrote: > > > > > > > > > > > > > > > > > > They introduce kernel bugs on purpose. Yesterday, I took a > > > > > > > > > > look on 4 > > > > > > > > > > accepted patches from Aditya and 3 of them added various > > > > > > > > > > severity security > > > > > > > > > > "holes". > > > > > > > > > > > > > > > > > > All contributions by this group of people need to be > > > > > > > > > reverted, if they > > > > > > > > > have not been done so already, as what they are doing is > > > > > > > > > intentional > > > > > > > > > malicious behavior and is not acceptable and totally > > > > > > > > > unethical. I'll > > > > > > > > > look at it after lunch unless someone else wants to do it… > > > > > > > > > > > > > > > > > > > > <snip> > > > > > > > > > > > > Academic research should NOT waste the time of a community. > > > > > > > > > > > > If you believe this behavior deserves an escalation, you can > > > > > > contact the Institutional Review Board (irb@umn.edu) at UMN to > > > > > > investigate whether this behavior was harmful; in particular, > > > > > > whether the research activity had an appropriate IRB review, and > > > > > > what safeguards prevent repeats in other communities. > > > > > > > > > > The huge advantage of being "community" is that we don't need to do > > > > > all > > > > > the above and waste our time to fill some bureaucratic forms with > > > > > unclear > > > > > timelines and results. > > > > > > > > > > Our solution to ignore all @umn.edu contributions is much more > > > > > reliable > > > > > to us who are suffering from these researchers. > > > > > > > > > > > > > <shrug>That's an easy thing to sidestep by just shifting to using a > > > > private email address.</shrug> > > > > > > If they just want to be jerks, yes. But they can't then use that type > > > of "hiding" to get away with claiming it was done for a University > > > research project as that's even more unethical than what they are doing > > > now. > > > > > > > There really is no alternative for maintainers other than to always be > > > > sceptical of patches submitted by people who are not known and trusted > > > > members of the community, and to scrutinise those patches with more > > > > care. > > > > > > Agreed, and when we notice things like this that were determined to be > > > bad, we have the ability to easily go back and rip the changes out and > > > we can slowly add them back if they are actually something we want to > > > do. > > > > > > Which is what I just did: > > > https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/ > > > > Greg, > > > > Did you push your series to the public git? I would like to add you a > > couple of reverts. > > Yes, it can be found here: > git@gitolite.kernel.org:/pub/scm/linux/kernel/git/gregkh/driver-core.git umn.edu-reverts > > You can send reverts in email if you want, whatever works best. > > > And do you have a list of not reverted commits? It will save us from > > doing same comparison of reverted/not reverted over and over. > > Below is the list that didn't do a simple "revert" that I need to look > at. I was going to have my interns look into this, there's no need to > bother busy maintainers with it unless you really want to, as I can't > tell anyone what to work on :) Ohh, you have interns, so even better. Thanks > > thanks, > > greg k-h > > ------------------------- > # commits that need to be looked at as a clean revert did not work > 990a1162986e > 58d0c864e1a7 > a068aab42258 > 8816cd726a4f > c705f9fc6a17 > 8b6fc114beeb > 169f9acae086 > 8da96730331d > f4f5748bfec9 > e08f0761234d > cb5173594d50 > 06d5d6b7f994 > d9350f21e5fe > 6f0ce4dfc5a3 > f0d14edd2ba4 > 46953f97224d > 3c77ff8f8bae > 0aab8e4df470 > 8e949363f017 > f8ee34c3e77a > fd21b79e541e > 766460852cfa > 41f00e6e9e55 > 78540a259b05 > 208c6e8cff1b > 7ecced0934e5 > 48f40b96de2c > 9aabb68568b4 > 2cc12751cf46 > 534c89c22e26 > 6a8ca24590a2 > d70d70aec963 > d7737d425745 > 3a10e3dd52e8 > d6cb77228e3a > 517ccc2aa50d > 07660ca679da > 0fff9bd47e13 > 6ade657d6125 > 2795e8c25161 > 4ec850e5dfec > 035a14e71f27 > 10010493c126 > 4280b73092fe > 5910fa0d0d98 > 40619f7dd3ef > 0a54ea9f481f > 44fabd8cdaaa > 02cc53e223d4 > c99776cc4018 > 7fc93f3285b1 > 6ae16dfb61bc > 9c6260de505b > eb8950861c1b > 46273cf7e009 > 89dfd0083751 > c9c63915519b > cd07e3701fa6 > 15b3048aeed8 > 7172122be6a4 > 47db7873136a > 58f5bbe331c5 > 6b995f4eec34 > 8af03d1ae2e1 > f16b613ca8b3 > 6009d1fe6ba3 > 8e03477cb709 > dc487321b1e6
On Wed, Apr 21, 2021 at 08:51:02AM -0400, Anna Schumaker wrote: > On Wed, Apr 21, 2021 at 2:07 AM Leon Romanovsky <leon@kernel.org> wrote: > > > > On Tue, Apr 20, 2021 at 01:10:08PM -0400, J. Bruce Fields wrote: > > > On Tue, Apr 20, 2021 at 09:15:23AM +0200, Greg KH wrote: > > > > If you look at the code, this is impossible to have happen. > > > > > > > > Please stop submitting known-invalid patches. Your professor is playing > > > > around with the review process in order to achieve a paper in some > > > > strange and bizarre way. > > > > > > > > This is not ok, it is wasting our time, and we will have to report this, > > > > AGAIN, to your university... > > > > > > What's the story here? > > > > Those commits are part of the following research: > > https://github.com/QiushiWu/QiushiWu.github.io/blob/main/papers/OpenSourceInsecurity.pdf > > This thread is the first I'm hearing about this. I wonder if there is > a good way of alerting the entire kernel community (including those > only subscribed to subsystem mailing lists) about what's going on? It > seems like useful information to have to push back against these > patches. IMHO, kernel users ML is good enough for that.
On Wed, Apr 21, 2021 at 05:15:26PM +0300, Leon Romanovsky wrote: > > This thread is the first I'm hearing about this. I wonder if there is > > a good way of alerting the entire kernel community (including those > > only subscribed to subsystem mailing lists) about what's going on? It > > seems like useful information to have to push back against these > > patches. > > IMHO, kernel users ML is good enough for that. The problem is that LKML is too high traffic for a lot of people to want to follow. There are some people who have used the kernel summit discuss list (previously ksummit-discuss@lists.linux-foundation.org, now ksummit@lists.linux.dev) as a place where most maintainers tend to be subscribed, although that's not really a guarantee, either. (Speaking of which, how to handle groups who submit patches in bad faith a good Maintainer Summit topic for someone to propose...) To give the devil his due, Prof. Kangjie Lu has reported legitimate security issues in the past (CVE-2016-4482, an information leak from the kernel stack in the core USB layer, and CVE-2016-4485, an information leak in the 802.2 networking code), and if one looks at his CV, he has a quite a few papers in the security area to his name. The problem is that Prof. Lu and his team seem to be unrepentant, and has some very... skewed... ideas over what is considered ethical, and acceptable behavior vis-a-vis the Kernel development community. The fact that the UMN IRB team believes that what Prof. Lu is doing isn't considered in scope for human experimentation means that there isn't any kind of institutional controls at UMN for this sort of behavior --- which is why a University-wide Ban may be the only right answer, unfortunately. - Ted
On Wed, Apr 21, 2021 at 11:48:46AM -0400, Theodore Ts'o wrote: > On Wed, Apr 21, 2021 at 05:15:26PM +0300, Leon Romanovsky wrote: > > > This thread is the first I'm hearing about this. I wonder if there is > > > a good way of alerting the entire kernel community (including those > > > only subscribed to subsystem mailing lists) about what's going on? It > > > seems like useful information to have to push back against these > > > patches. Heh, I've got this information from google news feed on my phone :) > > IMHO, kernel users ML is good enough for that. > > The problem is that LKML is too high traffic for a lot of people to > want to follow. I think Leon meant kernel.org users ML (users@linux.kernel.org). Along with ksummut-discuss it'll reach most maintainers, IMHO. > There are some people who have used the kernel summit discuss list > (previously ksummit-discuss@lists.linux-foundation.org, now > ksummit@lists.linux.dev) as a place where most maintainers tend to be > subscribed, although that's not really a guarantee, either. (Speaking > of which, how to handle groups who submit patches in bad faith a good > Maintainer Summit topic for someone to propose...) -- Sincerely yours, Mike.
> Below is the list that didn't do a simple "revert" that I need to look > at. I was going to have my interns look into this, there's no need to > bother busy maintainers with it unless you really want to, as I can't > tell anyone what to work on :) I'm not involved or affliated with the group or the kernel, but I'd like to make a suggestion: Do not revert umn.edu patches unconditionally. See below: According to the paper: > We submit the three patches using a randomGmail account to the Linux community andseek their feedback So while their behaviour regarding this practice may have been bad, I'd give them the benefit of doubt that they didn't want to actually introduce a bug. I.e. what they wrote: > we immediately notify themaintainers of the introduced UAF and request them to notgo ahead to apply the patch. > At the same time, we point out the correct fixing of the bug and provide our correct patch. > [...] All the UAF-introducing patches stayed only in the emailexchanges, without even becoming a Git commit in Linuxbranches TLDR: - The faulty patches were NOT from umn.edu accounts but from a gmail account - Only the corrected patches should have made it to the branches So while I would at least double-check that the last point is actually true, I believe reverting all umn.edu patches is wrong and actually (re-)introduces vulnerabilities or bugs which have been legitimately fixed (at least in good faith) And especially if the reverts do not apply cleanly on the current HEADs I think you might be wasting a lot of work/time, too. And yes, this aftermath makes it even worse what they did and excluding them from future contributions may make sense. But maybe reverting EVERYTHING is a bit to much here, especially if that doesn't even include the faulty stuff (assuming they are not plain lying in their paper, which I really doubt they would) Alex
On Tue, Apr 06, 2021 at 07:16:56PM -0500, Aditya Pakki wrote: > In gss_pipe_destroy_msg(), in case of error in msg, gss_release_msg > deletes gss_msg. The patch adds a check to avoid a potential double > free. > > Signed-off-by: Aditya Pakki <pakki001@umn.edu> > --- > net/sunrpc/auth_gss/auth_gss.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c > index 5f42aa5fc612..eb52eebb3923 100644 > --- a/net/sunrpc/auth_gss/auth_gss.c > +++ b/net/sunrpc/auth_gss/auth_gss.c > @@ -848,7 +848,8 @@ gss_pipe_destroy_msg(struct rpc_pipe_msg *msg) > warn_gssd(); > gss_release_msg(gss_msg); > } > - gss_release_msg(gss_msg); > + if (gss_msg) > + gss_release_msg(gss_msg); I know I am adding to the noise here, but it has to be said: gss_msg is assigned with struct gss_upcall_msg *gss_msg = container_of(msg, struct gss_upcall_msg, msg); and thus never NULL. Guenter > } > > static void gss_pipe_dentry_destroy(struct dentry *dir, > -- > 2.25.1 >
On Wed, Apr 21, 2021 at 08:34:59PM +0300, Mike Rapoport wrote: > On Wed, Apr 21, 2021 at 11:48:46AM -0400, Theodore Ts'o wrote: > > On Wed, Apr 21, 2021 at 05:15:26PM +0300, Leon Romanovsky wrote: > > > > This thread is the first I'm hearing about this. I wonder if there is > > > > a good way of alerting the entire kernel community (including those > > > > only subscribed to subsystem mailing lists) about what's going on? It > > > > seems like useful information to have to push back against these > > > > patches. > > Heh, I've got this information from google news feed on my phone :) > > > > IMHO, kernel users ML is good enough for that. > > > > The problem is that LKML is too high traffic for a lot of people to > > want to follow. > > I think Leon meant kernel.org users ML (users@linux.kernel.org). Along with > ksummut-discuss it'll reach most maintainers, IMHO. Exactly. Thanks > > > There are some people who have used the kernel summit discuss list > > (previously ksummit-discuss@lists.linux-foundation.org, now > > ksummit@lists.linux.dev) as a place where most maintainers tend to be > > subscribed, although that's not really a guarantee, either. (Speaking > > of which, how to handle groups who submit patches in bad faith a good > > Maintainer Summit topic for someone to propose...) > > -- > Sincerely yours, > Mike.
Hi Greg, On Wed, Apr 21, 2021 at 11:21 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Wed, Apr 21, 2021 at 11:07:11AM +0100, Sudip Mukherjee wrote: > > Hi Greg, > > > > On Wed, Apr 21, 2021 at 6:44 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > On Wed, Apr 21, 2021 at 08:10:25AM +0300, Leon Romanovsky wrote: > > > > On Tue, Apr 20, 2021 at 01:10:08PM -0400, J. Bruce Fields wrote: > > > > > On Tue, Apr 20, 2021 at 09:15:23AM +0200, Greg KH wrote: > > > > > > If you look at the code, this is impossible to have happen. > > > > > > > > > > <snip> > > > > > > They introduce kernel bugs on purpose. Yesterday, I took a look on 4 > > > > accepted patches from Aditya and 3 of them added various severity security > > > > "holes". > > > > > > All contributions by this group of people need to be reverted, if they > > > have not been done so already, as what they are doing is intentional > > > malicious behavior and is not acceptable and totally unethical. I'll > > > look at it after lunch unless someone else wants to do it... > > > > A lot of these have already reached the stable trees. I can send you > > revert patches for stable by the end of today (if your scripts have > > not already done it). > > Yes, if you have a list of these that are already in the stable trees, > that would be great to have revert patches, it would save me the extra > effort these mess is causing us to have to do... The patch series for all the stable branches should be with you now. But for others: https://lore.kernel.org/stable/YIEVGXEoeizx6O1p@debian/ for v5.11.y and other branches are sent as a reply to that mail. -- Regards Sudip
On Thu, Apr 22, 2021 at 09:10:53AM +0100, Sudip Mukherjee wrote: > Hi Greg, > > On Wed, Apr 21, 2021 at 11:21 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Wed, Apr 21, 2021 at 11:07:11AM +0100, Sudip Mukherjee wrote: > > > Hi Greg, > > > > > > On Wed, Apr 21, 2021 at 6:44 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > On Wed, Apr 21, 2021 at 08:10:25AM +0300, Leon Romanovsky wrote: > > > > > On Tue, Apr 20, 2021 at 01:10:08PM -0400, J. Bruce Fields wrote: > > > > > > On Tue, Apr 20, 2021 at 09:15:23AM +0200, Greg KH wrote: > > > > > > > If you look at the code, this is impossible to have happen. > > > > > > > > > > > > > <snip> > > > > > > > > They introduce kernel bugs on purpose. Yesterday, I took a look on 4 > > > > > accepted patches from Aditya and 3 of them added various severity security > > > > > "holes". > > > > > > > > All contributions by this group of people need to be reverted, if they > > > > have not been done so already, as what they are doing is intentional > > > > malicious behavior and is not acceptable and totally unethical. I'll > > > > look at it after lunch unless someone else wants to do it... > > > > > > A lot of these have already reached the stable trees. I can send you > > > revert patches for stable by the end of today (if your scripts have > > > not already done it). > > > > Yes, if you have a list of these that are already in the stable trees, > > that would be great to have revert patches, it would save me the extra > > effort these mess is causing us to have to do... > > The patch series for all the stable branches should be with you now. > > But for others: > https://lore.kernel.org/stable/YIEVGXEoeizx6O1p@debian/ for v5.11.y > and other branches are sent as a reply to that mail. Thank you, I now have them. I will be looking at them when I get the chance, and comparing them to what I end up getting merged into 5.13-rc1. greg k-h
On Wed, Apr 21, 2021 at 09:56:37AM -0400, J. Bruce Fields wrote: > On Wed, Apr 21, 2021 at 04:49:31PM +0300, Leon Romanovsky wrote: > > If you want to see another accepted patch that is already part of > > stable@, you are invited to take a look on this patch that has "built-in bug": > > 8e949363f017 ("net: mlx5: Add a missing check on idr_find, free buf") > > Interesting, thanks. Though looking at it now, I'm not actually seeing the bug--probably I'm overlooking something obvious. --b.
On Thu, Apr 22, 2021 at 03:39:50PM -0400, J. Bruce Fields wrote: > On Wed, Apr 21, 2021 at 09:56:37AM -0400, J. Bruce Fields wrote: > > On Wed, Apr 21, 2021 at 04:49:31PM +0300, Leon Romanovsky wrote: > > > If you want to see another accepted patch that is already part of > > > stable@, you are invited to take a look on this patch that has "built-in bug": > > > 8e949363f017 ("net: mlx5: Add a missing check on idr_find, free buf") > > > > Interesting, thanks. > > Though looking at it now, I'm not actually seeing the bug--probably I'm > overlooking something obvious. It was fixed in commit 31634bf5dcc4 ("net/mlx5: FPGA, tls, hold rcu read lock a bit longer") Thanks > > --b.
On Fri, Apr 23, 2021 at 08:25:28PM +0300, Leon Romanovsky wrote: > On Thu, Apr 22, 2021 at 03:39:50PM -0400, J. Bruce Fields wrote: > > On Wed, Apr 21, 2021 at 09:56:37AM -0400, J. Bruce Fields wrote: > > > On Wed, Apr 21, 2021 at 04:49:31PM +0300, Leon Romanovsky wrote: > > > > If you want to see another accepted patch that is already part of > > > > stable@, you are invited to take a look on this patch that has "built-in bug": > > > > 8e949363f017 ("net: mlx5: Add a missing check on idr_find, free buf") > > > > > > Interesting, thanks. > > > > Though looking at it now, I'm not actually seeing the bug--probably I'm > > overlooking something obvious. > > It was fixed in commit 31634bf5dcc4 ("net/mlx5: FPGA, tls, hold rcu read lock a bit longer") So is the "Fixes:" line on that commit wrong? It claims the bug was introduced by an earlier commit, ab412e1dd7db ("net/mlx5: Accel, add TLS rx offload routines"). Looks like Aditya Pakki's commit may have widened the race a little, but I find it a little hard to fault him for that. --b.
On Fri, Apr 23, 2021 at 02:07:27PM -0400, J. Bruce Fields wrote: > On Fri, Apr 23, 2021 at 08:25:28PM +0300, Leon Romanovsky wrote: > > On Thu, Apr 22, 2021 at 03:39:50PM -0400, J. Bruce Fields wrote: > > > On Wed, Apr 21, 2021 at 09:56:37AM -0400, J. Bruce Fields wrote: > > > > On Wed, Apr 21, 2021 at 04:49:31PM +0300, Leon Romanovsky wrote: > > > > > If you want to see another accepted patch that is already part of > > > > > stable@, you are invited to take a look on this patch that has "built-in bug": > > > > > 8e949363f017 ("net: mlx5: Add a missing check on idr_find, free buf") > > > > > > > > Interesting, thanks. > > > > > > Though looking at it now, I'm not actually seeing the bug--probably I'm > > > overlooking something obvious. > > > > It was fixed in commit 31634bf5dcc4 ("net/mlx5: FPGA, tls, hold rcu read lock a bit longer") > > So is the "Fixes:" line on that commit wrong? It claims the bug was > introduced by an earlier commit, ab412e1dd7db ("net/mlx5: Accel, add TLS > rx offload routines"). Yes, I think that Fixes line is misleading. > > Looks like Aditya Pakki's commit may have widened the race a little, but > I find it a little hard to fault him for that. We can argue about severity of this bug, but the whole paper talks about introduction of UAF bugs unnoticed. Thanks > > --b.
Have umn addresses been blocked from posting to kernel lists? Anyway: On Fri, Apr 23, 2021 at 10:29:52PM +0300, Leon Romanovsky wrote: > On Fri, Apr 23, 2021 at 02:07:27PM -0400, J. Bruce Fields wrote: > > On Fri, Apr 23, 2021 at 08:25:28PM +0300, Leon Romanovsky wrote: > > > On Thu, Apr 22, 2021 at 03:39:50PM -0400, J. Bruce Fields wrote: > > > > On Wed, Apr 21, 2021 at 09:56:37AM -0400, J. Bruce Fields wrote: > > > > > On Wed, Apr 21, 2021 at 04:49:31PM +0300, Leon Romanovsky wrote: > > > > > > If you want to see another accepted patch that is already part of > > > > > > stable@, you are invited to take a look on this patch that has "built-in bug": > > > > > > 8e949363f017 ("net: mlx5: Add a missing check on idr_find, free buf") > > > > > > > > > > Interesting, thanks. > > > > > > > > Though looking at it now, I'm not actually seeing the bug--probably I'm > > > > overlooking something obvious. > > > > > > It was fixed in commit 31634bf5dcc4 ("net/mlx5: FPGA, tls, hold rcu read lock a bit longer") > > > > So is the "Fixes:" line on that commit wrong? It claims the bug was > > introduced by an earlier commit, ab412e1dd7db ("net/mlx5: Accel, add TLS > > rx offload routines"). > > Yes, I think that Fixes line is misleading. > > > > > Looks like Aditya Pakki's commit may have widened the race a little, but > > I find it a little hard to fault him for that. > > We can argue about severity of this bug, but the whole paper talks about > introduction of UAF bugs unnoticed. Aditya Pakki points out in private mail that this patch is part of the work described in this paper: https://www-users.cs.umn.edu/~kjlu/papers/crix.pdf (See the list of patches in the appendix.) I mean, sure, I suppose they could have created that whole second line of research just as a cover to submit malicious patches, but I think we're running pretty hard into Occam's Razor at that point. --b.
On Fri, Apr 23, 2021 at 05:48:50PM -0400, J. Bruce Fields wrote: > Have umn addresses been blocked from posting to kernel lists? It is very unlikely. > > Anyway: > > On Fri, Apr 23, 2021 at 10:29:52PM +0300, Leon Romanovsky wrote: > > On Fri, Apr 23, 2021 at 02:07:27PM -0400, J. Bruce Fields wrote: > > > On Fri, Apr 23, 2021 at 08:25:28PM +0300, Leon Romanovsky wrote: > > > > On Thu, Apr 22, 2021 at 03:39:50PM -0400, J. Bruce Fields wrote: > > > > > On Wed, Apr 21, 2021 at 09:56:37AM -0400, J. Bruce Fields wrote: > > > > > > On Wed, Apr 21, 2021 at 04:49:31PM +0300, Leon Romanovsky wrote: > > > > > > > If you want to see another accepted patch that is already part of > > > > > > > stable@, you are invited to take a look on this patch that has "built-in bug": > > > > > > > 8e949363f017 ("net: mlx5: Add a missing check on idr_find, free buf") > > > > > > > > > > > > Interesting, thanks. > > > > > > > > > > Though looking at it now, I'm not actually seeing the bug--probably I'm > > > > > overlooking something obvious. > > > > > > > > It was fixed in commit 31634bf5dcc4 ("net/mlx5: FPGA, tls, hold rcu read lock a bit longer") > > > > > > So is the "Fixes:" line on that commit wrong? It claims the bug was > > > introduced by an earlier commit, ab412e1dd7db ("net/mlx5: Accel, add TLS > > > rx offload routines"). > > > > Yes, I think that Fixes line is misleading. > > > > > > > > Looks like Aditya Pakki's commit may have widened the race a little, but > > > I find it a little hard to fault him for that. > > > > We can argue about severity of this bug, but the whole paper talks about > > introduction of UAF bugs unnoticed. > > Aditya Pakki points out in private mail that this patch is part of the > work described in this paper: > > https://www-users.cs.umn.edu/~kjlu/papers/crix.pdf > > (See the list of patches in the appendix.) > > I mean, sure, I suppose they could have created that whole second line > of research just as a cover to submit malicious patches, but I think > we're running pretty hard into Occam's Razor at that point. Let's not speculate here. The lack of trust, due to unethical research that was done by UMN researchers, amount of bugs already introduced by @umn, and multiple attempts to repeat the same pattern (see Al Viro responses on patches like this SUNRPC patch) is enough to stop waste our time. Thanks > > --b.
On Fri, Apr 23, 2021 at 05:48:50PM -0400, J. Bruce Fields wrote:
> Have umn addresses been blocked from posting to kernel lists?
I don't see davem ever doing anything of that sort. I've no information
about what has really happened, but "Uni lawyers and/or HR telling them
to stop making anything that might be considered public statements" sounds
much more plausible...
Again, it's only a speculation and it might very well have been something
else, but out of those two variants... I'd put high odds on the latter vs
the former.
On Sat, Apr 24, 2021 at 06:34:45PM +0000, Al Viro wrote: > On Fri, Apr 23, 2021 at 05:48:50PM -0400, J. Bruce Fields wrote: > > Have umn addresses been blocked from posting to kernel lists? > > I don't see davem ever doing anything of that sort. I've no information > about what has really happened, but "Uni lawyers and/or HR telling them > to stop making anything that might be considered public statements" sounds > much more plausible... > > Again, it's only a speculation and it might very well have been something > else, but out of those two variants... I'd put high odds on the latter vs > the former. From private email: "Our UMN emails addresses are already banned from the mailing list." Also, I didn't get a copy of CA+EnHHSw4X+ubOUNYP2zXNpu70G74NN1Sct2Zin6pRgq--TqhA@mail.gmail.com through vger for some reason, and it didn't make it to lore.kernel.org either. In Greg's revert thread, Kangjie Lu's messages are also missing from the archives: https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/ ?? --b.
On Sat, Apr 24, 2021 at 05:34:54PM -0400, J. Bruce Fields wrote: > In Greg's revert thread, Kangjie Lu's messages are also missing from the > archives: > > https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/ > I'm going to guess it's one of two things. The first is that they are sending mail messages with HTML which is getting bounced; the other possibility is that some of the messages were sent only to Greg, and he added the mailing list back to the cc. So for exampple, message-id CA+EnHHSw4X+ubOUNYP2zXNpu70G74NN1Sct2Zin6pRgq--TqhA@mail.gmail.com isn't in lore, but Greg's reply: https://lore.kernel.org/linux-nfs/YH%2FfM%2FTsbmcZzwnX@kroah.com/ can be found in lore.kernel.org was presumably because the message where Aditya accused "wild accusations bordering on slander" and his claim that his patches were the fault of a "new static code analyzer" was sent only to Greg? Either that, or it was bounced because he sent it from gmail without suppressing HTML. - Ted
On Sat, Apr 24, 2021 at 08:41:27PM -0400, Theodore Ts'o wrote: > On Sat, Apr 24, 2021 at 05:34:54PM -0400, J. Bruce Fields wrote: > > In Greg's revert thread, Kangjie Lu's messages are also missing from the > > archives: > > > > https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/ > > > > I'm going to guess it's one of two things. The first is that they are > sending mail messages with HTML which is getting bounced; the other > possibility is that some of the messages were sent only to Greg, and > he added the mailing list back to the cc. > > So for exampple, message-id > CA+EnHHSw4X+ubOUNYP2zXNpu70G74NN1Sct2Zin6pRgq--TqhA@mail.gmail.com > isn't in lore, but Greg's reply: > > https://lore.kernel.org/linux-nfs/YH%2FfM%2FTsbmcZzwnX@kroah.com/ > > can be found in lore.kernel.org was presumably because the message > where Aditya accused "wild accusations bordering on slander" and his > claim that his patches were the fault of a "new static code analyzer" > was sent only to Greg? Either that, or it was bounced because he sent > it from gmail without suppressing HTML. I did not "add back" the mailing list, it looks like they sent email in html format which prevented it from hitting the public lists. I have the originals sent to me that shows the author intended it to be public. thanks, greg k-h
On Mon, Apr 26, 2021 at 09:36:05AM -0400, J. Bruce Fields wrote: > On Sun, Apr 25, 2021 at 08:29:53AM +0200, Greg KH wrote: > > On Sat, Apr 24, 2021 at 08:41:27PM -0400, Theodore Ts'o wrote: > > > On Sat, Apr 24, 2021 at 05:34:54PM -0400, J. Bruce Fields wrote: > > > > In Greg's revert thread, Kangjie Lu's messages are also missing from the > > > > archives: > > > > > > > > https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/ > > > > > > > > > > I'm going to guess it's one of two things. The first is that they are > > > sending mail messages with HTML which is getting bounced; the other > > > possibility is that some of the messages were sent only to Greg, and > > > he added the mailing list back to the cc. > > > > > > So for exampple, message-id > > > CA+EnHHSw4X+ubOUNYP2zXNpu70G74NN1Sct2Zin6pRgq--TqhA@mail.gmail.com > > > isn't in lore, but Greg's reply: > > > > > > https://lore.kernel.org/linux-nfs/YH%2FfM%2FTsbmcZzwnX@kroah.com/ > > > > > > can be found in lore.kernel.org was presumably because the message > > > where Aditya accused "wild accusations bordering on slander" and his > > > claim that his patches were the fault of a "new static code analyzer" > > > was sent only to Greg? Either that, or it was bounced because he sent > > > it from gmail without suppressing HTML. > > > > I did not "add back" the mailing list, it looks like they sent email in > > html format which prevented it from hitting the public lists. I have > > the originals sent to me that shows the author intended it to be public. > > Yes, the list cc's are all on there. > > It's multipart/alternative with equivalent plain text and html parts, > which appears to be gmail's default behavior. Is that really rejected > by default? Hah, when I sent that mail I quoted parts of the message including the Content-Type headers delineating the parts and got an immediate bounce saying "The message contains HTML subpart, therefore we consider it SPAM or Outlook Virus. TEXT/PLAIN is accepted". Which seems perfectly clear. OK, sorry for the noise! --b.
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c index 5f42aa5fc612..eb52eebb3923 100644 --- a/net/sunrpc/auth_gss/auth_gss.c +++ b/net/sunrpc/auth_gss/auth_gss.c @@ -848,7 +848,8 @@ gss_pipe_destroy_msg(struct rpc_pipe_msg *msg) warn_gssd(); gss_release_msg(gss_msg); } - gss_release_msg(gss_msg); + if (gss_msg) + gss_release_msg(gss_msg); } static void gss_pipe_dentry_destroy(struct dentry *dir,
In gss_pipe_destroy_msg(), in case of error in msg, gss_release_msg deletes gss_msg. The patch adds a check to avoid a potential double free. Signed-off-by: Aditya Pakki <pakki001@umn.edu> --- net/sunrpc/auth_gss/auth_gss.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)