A statement on the UMN mess

Posted Apr 24, 2021 1:02 UTC (Sat) by viro (subscriber, #7872)
In reply to: A statement on the UMN mess by pbonzini
Parent article: A statement on the UMN mess

OK, that went far enough. We have several things conflated here.

1) a low-quality paper a while ago, with no data, no experiment protocol, etc. The lack of data is a part of what's blowing the whole thing out of proportion - if they bothered to attach the list (or link to such) of SHA1 of commits that had come out of their experiment, or, better yet, maintained and provided the list of message-ids of all submissions, successful and not, this mess with blanket revert requests, etc. would've been far smaller (if happened at all)

2) one of the co-authors of that paper had been recently playing with a static analyzer of theirs and sent a bunch of patches coming out of that. Some of the patches were utter crap. Unfortunately, they got merged. Even more unfortunately, the author *still* have not bothered to send revert request for said garbage patches. _That_ is the worst (academical) sin he can be charged with in the entire story, AFAICS - if you publish something that turns out to be bogus, you really ought to publish correction or retraction as soon as possible. I am quite certain by now that patches had been crap in good faith; the odds of that being the penetration testing, take 2, are IMO very low.

3) patches claiming to be fixes are sometimes (and I would like to see the dataset behind that paper, exactly because it might provide interesting correlates) taken with only a cursory review. That was another commonality between their paper and the current set of patches, which is what got the mess going.

4) bulk revert request had, IMO, been wrong. Much too wide (UMN is a big school), wouldn't have caught any sock puppets, dramatically misstated what had been requested (it was actually a bulk *review* request put in that form) and got a lot of free publicity for the original paper, while we are at it.

5) results of review so far - nothing dramatic whatsoever. Nothing plausibly malicious, no wrongbots, overall more or less usual quality.

IOW, I don't see any reasons to block UMN folks, including the group involved with that paper.

A statement on the UMN mess

Posted Apr 24, 2021 2:02 UTC (Sat) by bfields (subscriber, #19510) [Link] (3 responses)

"one of the co-authors of that paper had been recently playing with a static analyzer of theirs and sent a bunch of patches coming out of that. Some of the patches were utter crap. Unfortunately, they got merged. Even more unfortunately, the author *still* have not bothered to send revert request for said garbage patches."

I think umn.edu mail is getting dropped by the kernel lists.

Which doesn't prevent them from responding to you personally. Still, it's not a good way to communicate that we'd like more followup from them.

By the way, https://www-users.cs.umn.edu/~kjlu/papers/crix.pdf does include more data in the appendices. Unfortunately the list is just by filename and line number. Still, knowing that and the authors it'd probably be doable to identify the patches. But that's from a couple years ago, it doesn't cover the latest set of patches.

A statement on the UMN mess

Posted Apr 24, 2021 3:00 UTC (Sat) by viro (subscriber, #7872) [Link] (2 responses)

First of all, I *really* hope umn.edu mail does not get dropped by kernel.org lists; if it is, that should be fixed ASAFuckingP. As for the rest of it... Revert for 0c85a7e87465 ought to have been sent to Linus. By Aditya. As soon as he'd taken a good look at the patch he'd sent - say, on Apr 9, when the first replies started to arrive.

Look, you know and I know that shit happens; so does Linus - we'd all been in situations when embarrassingly bad patch got sent, applied and pushed out. I do not believe that Linus would sit on the author-sent revert request for days - and no such revert had been applied so far.

It doesn't need to be sent (or Cc'd) to any public lists - mail to Linus and davem with anything along the lines of "$commit had been garbage; fortunately it's harmless, but it's completely bogus and should've been caught before sending it out; please revert it" would've taken care of the retraction just fine. _Not_ doing that is really bad form.

Seriously, check that commit. The first chunk: "let's zero a local variable immediately upstream of return, lest we get double-free"; the second one: "we are slightly downstream of spin_unlock_irqrestore(&rm->m_rs_lock, flags); better check that we hadn't somehow gotten through it with rm == NULL" (in the second one rm comes from list_entry(), BTW). It's that bogus...

As for the missing data, I'd been referring to the recent paper on UAF injection and experiment described (FSVO) in there.

A statement on the UMN mess

Posted Apr 24, 2021 21:54 UTC (Sat) by bfields (subscriber, #19510) [Link] (1 responses)

"Seriously, check that commit."

This, yes:

https://lore.kernel.org/lkml/20210407000913.2207831-1-pak...

and this one:

https://lore.kernel.org/linux-nfs/20210421225240.GA117423...

was just as odd. I'd be curious to hear what happened. Agreed that they should have followed up.

Half the world seems too angry to think straight at this point.

"As for the missing data, I'd been referring to the recent paper on UAF injection and experiment described (FSVO) in there."

Understood. It doesn't give me much confidence that they're not willing to list those patches.

A statement on the UMN mess

Posted Apr 25, 2021 0:08 UTC (Sun) by viro (subscriber, #7872) [Link]

Regarding the static analyzers in general... IME they are best used to find potentially fishy places, so those who use those could see the low-hanging fruit for code audit. Having them generate patches is a bad idea - the task is not quite AI-complete, but in practice you can't let them run unfiltered (as these examples show) and submitter's filtering throughput ends up being the bottleneck; automating the generation of patches doesn't help - it's not the rate-limiting stage.

A statement on the UMN mess

Posted Apr 24, 2021 8:47 UTC (Sat) by pbonzini (subscriber, #60935) [Link]

> bulk revert request had, IMO, been wrong. Much too wide (UMN is a big school), wouldn't have caught any sock puppets, dramatically misstated what had been requested

Agreed. But my point is: knowing that this static analyzer from UMN is crap, even if they improve it it will be a while before maintainers start reviewing UMN static analyzer fixes with any interest.

Sending crap patches has happened to everyone, accepting them too. But it's one thing to break some eggs while making an omelette, it's another thing to carelessly drop the entire box of eggs on the floor.

Banning is theatre, loss of trust is not.