5 ms·

It’s a good observation. So for example we have a 2019 commit by Wenwen Wang being reverted. Wenwen is now at UGA. The commit is “ALSA: usx2y fix a double free

It’s a good observation. So for example we have a 2019 commit by Wenwen Wang being reverted. Wenwen is now at UGA. The commit is “ALSA: usx2y fix a double free bug” review by Takashi Iwai confirms it’s a good fix. I don’t agree with Greg K-H threatening to trash Wenwen’s work, his reputation, and add bugs to the Linux kernel out of spite toward researchers than Wenwen hasn’t worked with in years. Note that Aditya Pakki denies being a part of the hypocrite commits on email, maintains his denial in the apology, and has written papers on static analysis. Also, vast majority of those commits are being re-reviewed and found good. Greg Kroah-Hartman accused him of intentionally submitting bad commits. That accusation appears false. Greg should apologize to Wenwen and Aditaya. This is a broad brush. It’s hitting a bunch of good commits and we know the 3 hypocrite commits aren’t there .

rincebrain 5y ago
The complaints, depending on who you were listening to, were that the patches were superfluous (cf. [1], where Aditya apparently later claimed "The patch is garbage") or contained security flaws (cf. [2] for one such allegation) , whether deliberate or accidental, and that too many of them had been accepted without enough review by dint of being "trivial fixes". (One claim, for example, was "I took a look on 4 accepted patches from Aditya and 3 of them added various severity security "holes"." [3], which, if false, should certainly probably provoke an apology.) Assuming [3] came from a reasonably trustworthy source after Greg's initial distrust and frustration, combined with Aditya's reply of being extremely offended that anyone would doubt his work (which, if we assume for a moment he wasn't aware of the researchers' prior work poisoning LKML's opinion of the lab, could be a reasonable reaction), it's not surprising that his conclusion was "rip out all the patches for now and re-review them as we have time". People keep overlooking that the plan was never "permanently remove the patches", it was always "remove the patches for now and then re-review them all". And now it's working as expected - people are re-reviewing the patches proposed for removal and going "hey this is fine", "hey this is harmless", or occasionally, probably "hey this is bad". (I have not read anywhere near all the replies to the thread, I'm just assuming that the people who were complaining about the patches were also operating in good faith and not just making up complaints.) I don't really see another reasonable way to have acted if you suspect a group of people has been generating and getting committed poor patches, whether out of malice or ignorance, than removing them for now and re-reviewing them. If it turns out that the patches were (probably, since I don't think there's any absolute confidence to be had here) merely bad and not malicious, then sure, an apology would be warranted for claiming malice where there was none. (And for those who don't think he ever claimed malice, like I did when I started writing this reply, see [4], specifically '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.') But I don't think "okay we need to re-review all these patches (and the usual thing to do if we need to re-review patches is remove them for now)" warrants an apology in itself. [1] - https://github.com/torvalds/linux/commit/799bac5512188522213e2d7eb78ca7094dfdf30c https://github.com/torvalds/linux/commit/799bac5512188522213... [2] - https://lore.kernel.org/linux-nfs/YIAta3cRl8mk%2FRkH@unreal/ https://lore.kernel.org/linux-nfs/YIAta3cRl8mk%2FRkH@unreal/ [3] - https://lore.kernel.org/linux-nfs/YH+zwQgBBGUJdiVK@unreal/ https://lore.kernel.org/linux-nfs/YH+zwQgBBGUJdiVK@unreal/ [4] - https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/ https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh...
- nuclearnice1 5y ago
  Great detailed specific reply. In the end, we largely agree. It is exactly the accusation of malice when what occurred was normal error that deserves an apology. You teased that out well. As you note, we don’t have certainty. But we observe: Aditya is not an author on the hypocrite commits paper. Aditya withdrew his “garbage” commit when made aware it was not correct. Aditya is author of other papers on static analysis. Aditya’s other commits have generally survived this bulk review. Leon Romanovksy has offered no follow up for his claim of security holes nor explained his claim that the commits are part of the hypocrite commits research. Aditya maintains his claims even as other members of UMN have explained their role in the hypocrite commits. Aditya’s commits come from a UMN address where as hypocrite commits came from gmail. These observations make it overwhelmingly likely that Aditya’s commits are from a good faith somewhat buggy static analysis effort. This poor guy has worked for years on the Linux kernel and Greg Kroah-Hartman’s thoughtless rush to judgement threatened all that effort. Greg should apologize. Leon Romanovksy should too. They mixed up and couple things and got carried away by their emotions. It happens. They should work to undo the damage.
  - rincebrain 5y ago
    > Aditya withdrew his “garbage” commit when made aware it was not correct. It seems like he withdrew it in response to stumbling over [2], wherein Al Viro points out that the correct thing to do with buggy commits is to request they be reverted. (Based on, among other things, the fact that said LWN post is cited in the revert.) > Leon Romanovksy has offered no follow up for his claim of security holes nor explained his claim that the commits are part of the hypocrite commits research. He did offer the patch I cited as "[2]" in my prior reply, as well as pointing out [1] the commit where they reworked the buggy logic from it. One commit does not a sinner make, but it's not correct to say he offered no data. > They mixed up and couple things and got carried away by their emotions. I'm not the biggest fan of GregKH for other reasons [3], but other than maybe explicitly requiring and verifying a list of broken commits beforehand, I'm not sure what I would have wanted him to do differently. If you have prior reason to suspect a group of behaving maliciously, and someone you trust attests that they appear to have behaved maliciously, what do you do differently? "I promise I'm not part of that research" doesn't work if you think the group might lie, and as I've pointed out other times, the only ones caught in the temporary patch removal were members of the relevant lab, and I further claim that "patches temporarily removed for re-examining" is not that severe a punishment. [1] - https://lore.kernel.org/linux-nfs/YIMDCNx4q6esHTYt@unreal/ https://lore.kernel.org/linux-nfs/YIMDCNx4q6esHTYt@unreal/ [2] - https://lwn.net/Articles/854319/ https://lwn.net/Articles/854319/ [3] - https://marc.info/?l=linux-kernel&m=154714516832389 https://marc.info/?l=linux-kernel&m=154714516832389

rincebrain 5y ago

The complaints, depending on who you were listening to, were that the patches were superfluous (cf. [1], where Aditya apparently later claimed "The patch is garbage") or contained security flaws (cf. [2] for one such allegation) , whether deliberate or accidental, and that too many of them had been accepted without enough review by dint of being "trivial fixes". (One claim, for example, was "I took a look on 4 accepted patches from Aditya and 3 of them added various severity security "holes"." [3], which, if false, should certainly probably provoke an apology.) Assuming [3] came from a reasonably trustworthy source after Greg's initial distrust and frustration, combined with Aditya's reply of being extremely offended that anyone would doubt his work (which, if we assume for a moment he wasn't aware of the researchers' prior work poisoning LKML's opinion of the lab, could be a reasonable reaction), it's not surprising that his conclusion was "rip out all the patches for now and re-review them as we have time". People keep overlooking that the plan was never "permanently remove the patches", it was always "remove the patches for now and then re-review them all". And now it's working as expected - people are re-reviewing the patches proposed for removal and going "hey this is fine", "hey this is harmless", or occasionally, probably "hey this is bad". (I have not read anywhere near all the replies to the thread, I'm just assuming that the people who were complaining about the patches were also operating in good faith and not just making up complaints.) I don't really see another reasonable way to have acted if you suspect a group of people has been generating and getting committed poor patches, whether out of malice or ignorance, than removing them for now and re-reviewing them. If it turns out that the patches were (probably, since I don't think there's any absolute confidence to be had here) merely bad and not malicious, then sure, an apology would be warranted for claiming malice where there was none. (And for those who don't think he ever claimed malice, like I did when I started writing this reply, see [4], specifically '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.') But I don't think "okay we need to re-review all these patches (and the usual thing to do if we need to re-review patches is remove them for now)" warrants an apology in itself. [1] - https://github.com/torvalds/linux/commit/799bac5512188522213e2d7eb78ca7094dfdf30c https://github.com/torvalds/linux/commit/799bac5512188522213... [2] - https://lore.kernel.org/linux-nfs/YIAta3cRl8mk%2FRkH@unreal/ https://lore.kernel.org/linux-nfs/YIAta3cRl8mk%2FRkH@unreal/ [3] - https://lore.kernel.org/linux-nfs/YH+zwQgBBGUJdiVK@unreal/ https://lore.kernel.org/linux-nfs/YH+zwQgBBGUJdiVK@unreal/ [4] - https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/ https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh...

nuclearnice1 5y ago
Great detailed specific reply. In the end, we largely agree. It is exactly the accusation of malice when what occurred was normal error that deserves an apology. You teased that out well. As you note, we don’t have certainty. But we observe: Aditya is not an author on the hypocrite commits paper. Aditya withdrew his “garbage” commit when made aware it was not correct. Aditya is author of other papers on static analysis. Aditya’s other commits have generally survived this bulk review. Leon Romanovksy has offered no follow up for his claim of security holes nor explained his claim that the commits are part of the hypocrite commits research. Aditya maintains his claims even as other members of UMN have explained their role in the hypocrite commits. Aditya’s commits come from a UMN address where as hypocrite commits came from gmail. These observations make it overwhelmingly likely that Aditya’s commits are from a good faith somewhat buggy static analysis effort. This poor guy has worked for years on the Linux kernel and Greg Kroah-Hartman’s thoughtless rush to judgement threatened all that effort. Greg should apologize. Leon Romanovksy should too. They mixed up and couple things and got carried away by their emotions. It happens. They should work to undo the damage.
- rincebrain 5y ago
  > Aditya withdrew his “garbage” commit when made aware it was not correct. It seems like he withdrew it in response to stumbling over [2], wherein Al Viro points out that the correct thing to do with buggy commits is to request they be reverted. (Based on, among other things, the fact that said LWN post is cited in the revert.) > Leon Romanovksy has offered no follow up for his claim of security holes nor explained his claim that the commits are part of the hypocrite commits research. He did offer the patch I cited as "[2]" in my prior reply, as well as pointing out [1] the commit where they reworked the buggy logic from it. One commit does not a sinner make, but it's not correct to say he offered no data. > They mixed up and couple things and got carried away by their emotions. I'm not the biggest fan of GregKH for other reasons [3], but other than maybe explicitly requiring and verifying a list of broken commits beforehand, I'm not sure what I would have wanted him to do differently. If you have prior reason to suspect a group of behaving maliciously, and someone you trust attests that they appear to have behaved maliciously, what do you do differently? "I promise I'm not part of that research" doesn't work if you think the group might lie, and as I've pointed out other times, the only ones caught in the temporary patch removal were members of the relevant lab, and I further claim that "patches temporarily removed for re-examining" is not that severe a punishment. [1] - https://lore.kernel.org/linux-nfs/YIMDCNx4q6esHTYt@unreal/ https://lore.kernel.org/linux-nfs/YIMDCNx4q6esHTYt@unreal/ [2] - https://lwn.net/Articles/854319/ https://lwn.net/Articles/854319/ [3] - https://marc.info/?l=linux-kernel&m=154714516832389 https://marc.info/?l=linux-kernel&m=154714516832389