Re: [GIT PULL] SafeSetID LSM changes for 5.3

[Micah Morton <mortonm@chromium.org>]

On Tue, Jul 16, 2019 at 12:06 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> 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.

Yes, I have a patch for this but was told it would be better to send
the patch through my tree rather than the security tree. I can send a
pull request for that.

>
> > 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

Got it.

>
>    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

This was not in linux-next, but was tested by Jann on a Chrome OS
device. There's also the selftest for this code. But I can send
non-trivial stuff to linux-next first next time.

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

Got it.

>
>    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".

Makes sense. The development was not actually done on that kernel, I
just grabbed that random kernel for committing the changes on top of
(these changes were developed a little while ago, but they're all self
contained to the SafeSetID LSM), but I'll pick a stable one next time.

>
>  - 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.

Thanks for the pointer. I had not seen that yet.

>
> 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.

Yep, makes sense.

>
>                    Linus

Thanks!