[GTALUG] Linus Torvalds Responds to Linux Banning University of Minnesota

[D. Hugh Redelmeier]

| From: Alvin Starr via talk <talk at gtalug.org>

| If the zdnet report is to be believed then There was at least one attempt to
| insert code after being found out and asked to stop.
| 
| https://www.zdnet.com/article/greg-kroah-hartman-bans-university-of-minnesota-from-linux-development-for-deliberately-buggy-patches/

See:
<https://lore.kernel.org/linux-nfs/20210407001658.2208535-1-pakki001@umn.edu/>

I don't think that Steven J. Vaughan-Nichols' interpretation is
correct (it seems to be GKH's).  If you look at the email exchange in
question, the "attempt to insert code" was an attempt to submit a real
bug-fix, not an attempt to add a bug.  But:

- the fix was to a bug that didn't exist.  Careful reading of the
  surrounding code shows that the problem addressed could not happen.

- it is hard to understand leaks and non-leaks, so this submission
  only shows that Pakki is not yet a good kernel programmer.

- it does not introduce a vulnerability

Here's the original function (from a perhaps different version of the
kernel):

static void
gss_pipe_destroy_msg(struct rpc_pipe_msg *msg)
{
	struct gss_upcall_msg *gss_msg = container_of(msg, struct gss_upcall_msg, msg);

	if (msg->errno < 0) {
		refcount_inc(&gss_msg->count);
		gss_unhash_msg(gss_msg);
		if (msg->errno == -ETIMEDOUT)
			warn_gssd();
		gss_release_msg(gss_msg);
	}
	gss_release_msg(gss_msg);
}

The patch submitted by Pakki was:

--- 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 don't see how gss_msg could be null, even just reading this code.
So the added test doesn't change anything.  No bug fixed.  No bug
introduced.

This certainly doesn't add a vulnerability.

But I think that the following code would work and be simpler.  Note:
my suggestion is just a guess.  I don't know the semantics of the
functions called.

static void
gss_pipe_destroy_msg(struct rpc_pipe_msg *msg)
{
	struct gss_upcall_msg *gss_msg = container_of(msg, struct gss_upcall_msg, msg);

	if (msg->errno < 0) {
		gss_unhash_msg(gss_msg);
		if (msg->errno == -ETIMEDOUT)
			warn_gssd();
	}
	gss_release_msg(gss_msg);
}

Something like this was suggested in the LKML thread.