Re: [GIT PULL] SafeSetID LSM changes for 5.3

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Micah Morton <mortonm@chromium.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-security-module <linux-security-module@vger.kernel.org>
Subject: Re: [GIT PULL] SafeSetID LSM changes for 5.3
Date: Tue, 16 Jul 2019 12:06:11 -0700	[thread overview]
Message-ID: <CAHk-=whY1J-LFvTa8ihiPNRSv1dwxPk9ycPCEhdcjsk7c=tGAw@mail.gmail.com> (raw)
In-Reply-To: <CAJ-EccPGqp4PmRkFk505QhDKHWn-ajxS0__Nk9VS32jV_+3Y2A@mail.gmail.com>

On Mon, Jul 15, 2019 at 9:05 AM Micah Morton <mortonm@chromium.org> wrote:
>
> I'm maintaining the new SafeSetID LSM and was told to set up my own
> tree for sending pull requests rather than sending my changes through
> James Morris and the security subsystem tree.

Yes. It would be good if you also added yourself to the MAINTAINERS
file. Right now there's no entry for security/safesetid at all.

> This is my first time doing one of these pull requests so hopefully I
> didn't screw something up.

So a couple of notes:

 - *please* don't rebase your work in the day before

   Was this in linux-next? was this tested at all? Hard to tell, since
it was rebased recently, so for all I know it's all completely new

 - don't use a random kernel-of-the-day as the base for development

   This is related to the rebasing issue, but is true even if you
don't rebase. There is no way that it was a good idea to pick my
random - possibly completely broken - kernel from Sunday afternoon in
the middle of a merge window as a base for development.

   If you start development, or if you have to rebase (for some *good*
reason) you need to do so on a good stable base, not on the quick-sand
that is "random kernel of the day during the busiest merge activity".

 - Please use the "git pull-request" format and then add any extra
notes you feel are necessary

   Yes, your pull request is *almost* git pull-request, but you seem
to have actively removed whitespace making it almost illegible. It's
really hard to pick out the line that has the actual git repository
address, because it's basically hidden inside one big blob of text.

I've pulled this as-is since it's the first time, but I expect better next time.

There are various resources on some cleanliness issues, and people
fairly recently tried to combine it under

   Documentation/maintainer/rebasing-and-merging.rst

which covers at least the basics on why not to rebase etc.

And if you *do* end up rebasing, consider the end result "untested",
so then it should have been done before the merge window even started,
and the rebased branch should have been in linux-next. And not sent to
me the very next day.

                   Linus