[GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[Micah Morton]

Hi Linus,

You mentioned a couple weeks ago it would be good if I added myself to
the MAINTAINERS file for the SafeSetID LSM. Here's the pull request
for v5.3.

Thanks!
--
The following changes since commit 179757afbef5f64b9bd25e6161f72fc1a52a8f2e:

  Merge commit 'v5.3-rc2^0' (2019-07-31 13:45:16 -0700)

are available in the Git repository at:

  https://github.com/micah-morton/linux.git tags/safesetid-maintainers-5.3-rc2

for you to fetch changes up to 7e20e910eabdf0af90fd10e712f15b413be8e135:

  Add entry in MAINTAINERS file for SafeSetID LSM (2019-07-31 13:58:11 -0700)

----------------------------------------------------------------
Add entry in MAINTAINERS file for SafeSetID LSM.

Has not had any bake time or testing, since its just changes to a text file.

----------------------------------------------------------------
Micah Morton (1):
      Add entry in MAINTAINERS file for SafeSetID LSM

 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[Linus Torvalds]

On Wed, Jul 31, 2019 at 2:30 PM Micah Morton <mortonm@chromium.org> wrote:
>
> You mentioned a couple weeks ago it would be good if I added myself to
> the MAINTAINERS file for the SafeSetID LSM. Here's the pull request
> for v5.3.

There's a lot more than the maintainer ID in there. You've rebased old
patches that I already had etc:

  Jann Horn (10):
      LSM: SafeSetID: fix pr_warn() to include newline
      LSM: SafeSetID: fix check for setresuid(new1, new2, new3)
      LSM: SafeSetID: refactor policy hash table
      LSM: SafeSetID: refactor safesetid_security_capable()
      LSM: SafeSetID: refactor policy parsing
      LSM: SafeSetID: fix userns handling in securityfs
      LSM: SafeSetID: rewrite userspace API to atomic updates
      LSM: SafeSetID: add read handler
      LSM: SafeSetID: verify transitive constrainedness
      LSM: SafeSetID: fix use of literal -1 in capable hook

  Micah Morton (2):
      Merge commit 'v5.3-rc2^0'
      Add entry in MAINTAINERS file for SafeSetID LSM

Not pulled.

                  Linus

Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[Micah Morton]

Sorry about that. To fix it I did a "git reset hard" to before any of
those commits by Jann Horn, then fast-forwarded to the v5.3-rc2 tag
and force pushed that to my origin/master then pushed a new branch up
with my MAINTAINERS file changes. Hopefully this is a valid fix.

--
The following changes since commit 609488bc979f99f805f34e9a32c1e3b71179d10b:

  Linux 5.3-rc2 (2019-07-28 12:47:02 -0700)

are available in the Git repository at:

  https://github.com/micah-morton/linux.git
  tags/safesetid-maintainers-correction-5.3-rc2

for you to fetch changes up to fc5b34a35458314df1dd00281f6e41f419581aa9:

  Add entry in MAINTAINERS file for SafeSetID LSM (2019-08-01 10:30:57 -0700)

----------------------------------------------------------------
Add entry in MAINTAINERS file for SafeSetID LSM.

Has not had any bake time or testing, since its just changes to a text file.

----------------------------------------------------------------
Micah Morton (1):
      Add entry in MAINTAINERS file for SafeSetID LSM

 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

On Thu, Aug 1, 2019 at 6:25 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Jul 31, 2019 at 2:30 PM Micah Morton <mortonm@chromium.org> wrote:
> >
> > You mentioned a couple weeks ago it would be good if I added myself to
> > the MAINTAINERS file for the SafeSetID LSM. Here's the pull request
> > for v5.3.
>
> There's a lot more than the maintainer ID in there. You've rebased old
> patches that I already had etc:
>
>   Jann Horn (10):
>       LSM: SafeSetID: fix pr_warn() to include newline
>       LSM: SafeSetID: fix check for setresuid(new1, new2, new3)
>       LSM: SafeSetID: refactor policy hash table
>       LSM: SafeSetID: refactor safesetid_security_capable()
>       LSM: SafeSetID: refactor policy parsing
>       LSM: SafeSetID: fix userns handling in securityfs
>       LSM: SafeSetID: rewrite userspace API to atomic updates
>       LSM: SafeSetID: add read handler
>       LSM: SafeSetID: verify transitive constrainedness
>       LSM: SafeSetID: fix use of literal -1 in capable hook
>
>   Micah Morton (2):
>       Merge commit 'v5.3-rc2^0'
>       Add entry in MAINTAINERS file for SafeSetID LSM
>
> Not pulled.
>
>                   Linus

Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[Linus Torvalds]

On Thu, Aug 1, 2019 at 11:11 AM Micah Morton <mortonm@chromium.org> wrote:
>
> Add entry in MAINTAINERS file for SafeSetID LSM.

So I've pulled this now.

However, I have to say that I'm now very nervous about future pulls,
simply because the last one had basically everything that can be wrong
be wrong.

Random rebasing of existing commits, a random merge with no sane merge
message.. All complete no-no's.

So I will have to remember to be careful when pulling from you, and
you need to get into a habit of not doing those things.

One very powerful git tool is "gitk". It's just a good idea to use it
to *visualize* to yourself what it is you actually have. Do something
like

    git fetch linus   (or "upstream" or "origin" or whatever your
remote branch for my tree is called)
    gitk linus/master..

which should show you very clearly what you have that is not in my
tree, and should show any odd merges etc.

Just doing "git diff" doesn't show garbage _history_, it only shows
the differences between the two states. There can be crazy bad history
that doesn't show up in the diff, exactly because you had duplicate
commits or something like that.

             Linus

Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[Linus Torvalds]

On Thu, Aug 1, 2019 at 11:11 AM Micah Morton <mortonm@chromium.org> wrote:
>
> The following changes since commit 609488bc979f99f805f34e9a32c1e3b71179d10b:
>
>   Linux 5.3-rc2 (2019-07-28 12:47:02 -0700)
>
> are available in the Git repository at:
>
>   https://github.com/micah-morton/linux.git
>   tags/safesetid-maintainers-correction-5.3-rc2

Hmm.

This pull request was apparently not caught by pr-tracker-bot for some
reason, so it didn't get the automated "this has been pulled" message.

I'm not entirely sure why - it was cc'd to lkml, and I see it on lore as

   https://lore.kernel.org/lkml/CAJ-EccOqmmrf2KPb7Z7NU6bF_4W1XUawLLy=pLekCyFKqusjKQ@mail.gmail.com/

so the email itself made it through the system. And it has "GIT PULL"
in the subject line, so the pr-tracker-bot should have looked at it.

I see a couple of _potential_ reasons why it might have been overlooked:

 - maybe the "--" marker after your explanation made pr-tracker-bot go
"oh, the rest is just a signature"

 - the fact that the git link looks more like a regular web thing, and
the branch name is on another line. Does pr-tracker-bot only trigger
on kernel.org things?

 - maybe pr-tracker-bot ignores follow-up emails with "Re:" in the subject?

but it could be something else too.

Adding Konstantin to the participants, since he knows the magic.

This is not a big deal, and I have probably missed a lot of other
cases where the pr-tracker-bot doesn't react to pull requests, but I
really like how it gives a heads-up to people about their pulls
without me having to do anything extra, so I generally try to look for
failures when I can.

Konstantin?

                  Linus

Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[Konstantin Ryabitsev]

On Sun, Aug 04, 2019 at 10:47:54AM -0700, Linus Torvalds wrote:
> - maybe pr-tracker-bot ignores follow-up emails with "Re:" in the 
> subject?

Yes, this is the culprit. Here are the matching regexes:

https://git.kernel.org/pub/scm/linux/kernel/git/mricon/korg-helpers.git/tree/pr-tracker-bot.py#n41

Normally, pull requests don't come in as replies -- this is the first 
one that got missed in months, to my knowledge.

I can change it to be more permissive -- the only concern would be that 
we would be scanning more bodies, and there *might* be a chance where we 
end up tracking the wrong message if someone uses Outlook-style replies 
(fully quoted messages without '>') and those replies arrive before the 
original message. Both of these are very unlikely to happen (in fact, I 
believe using Outlook-style replies on LKML would result in mass 
outrage).

So, just let me know if you'd like me to make this change and I'll 
modify the regex to be something like '^\S*:?\s*\[GIT' that should match 
most permutations of Re/Aw/etc.

-K

Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[Linus Torvalds]

On Mon, Aug 5, 2019 at 7:28 AM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
>
> On Sun, Aug 04, 2019 at 10:47:54AM -0700, Linus Torvalds wrote:
> > - maybe pr-tracker-bot ignores follow-up emails with "Re:" in the
> > subject?
>
> Yes, this is the culprit. Here are the matching regexes:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/mricon/korg-helpers.git/tree/pr-tracker-bot.py#n41
>
> Normally, pull requests don't come in as replies -- this is the first
> one that got missed in months, to my knowledge.

Most pull requests certainly are proper starts of threads. And I
generally wish they were, because I know I myself tend to skim over
emails much more quickly if it's some old discussion that I either
consider solved or where somebody else is handling it, so if I see a
pull request in the middle of a thread, it's much more likely that I'd
miss it.

It does happen, though. Not just in situations like this, where I
replied to the original pull request with some reason for why I
wouldn't pull it, and then the fixed pull came in as part of the
thread.

Al Viro does that to me occasionally, for example. Some discussion
about a vfs problem, and then the pull request is in the middle of
that thread. You can see an example of that here:

     https://lore.kernel.org/lkml/20180125002151.GR13338@ZenIV.linux.org.uk/

although in that case it was Davem who merged it (in merge commit 8ec59b44a006.

Of course, pr-tracker-bot wouldn't have noticed that one anyway,
because it also doesn't have "GIT PULL" or anything like that in the
subject line at all. So maybe it's not a great example.

I don't know if it's worth changing the pr-tracker-bot rules. I *do*
think that the whole unquoted

   for you to fetch changes up to [hex string]

is by far the strongest single signal for a pull request, but it's not
clear that it's worth spending a lot of CPU time looking for that
unless you have a strong signal in the subject line.

So I consider this "solved", and maybe people should just realize that
they won't get the automated responses unless they do everything just
right.

                  Linus

Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[Konstantin Ryabitsev]

On Mon, Aug 05, 2019 at 11:20:59AM -0700, Linus Torvalds wrote:
>I don't know if it's worth changing the pr-tracker-bot rules. I *do*
>think that the whole unquoted
>
>   for you to fetch changes up to [hex string]
>
>is by far the strongest single signal for a pull request, but it's not
>clear that it's worth spending a lot of CPU time looking for that
>unless you have a strong signal in the subject line.

The way we do it currently is by hooking into public-inbox where the 
email subject is in the commit log. So for us to grab all new subjects 
it's a single git call, whereas getting the message body requires a git 
call per message. This is why we pre-filter by subject, as it's a cheap 
way to avoid needing to issue hundreds of git calls looking for possible 
matches in message bodies.

>So I consider this "solved", and maybe people should just realize that
>they won't get the automated responses unless they do everything just
>right.

Would you consider recording the message-id of the pull request as part 
of the commit message? This would be a sure way for us to be able to 
catch all possible cases. In fact, this would allow me to throw out most 
of the bot logic, as it would become unnecessary. E.g. the merge commit 
would look like:

Merge tag 'foo' of git://git.kernel.org/bar

Pull foo features

 * foo
 * bar
 * baz

Link: https://lore.kernel.org/r/<message-id>


However, I suspect that getting message-ids for all your pull requests 
would significantly complicate your workflow.

-K

Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[Linus Torvalds]

On Mon, Aug 5, 2019 at 12:11 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
>
> However, I suspect that getting message-ids for all your pull requests
> would significantly complicate your workflow.

Yeah, that would be a noticeable annoyance. If I were to process pull
requests the way I used to process emailed patches (ie "git am -s" on
a mailbox) that would be a natural thing to perhaps do, but it's not
at all how it ends up working. Having to save the pull request email
to then process it with some script would turn it into a chore.

I think the pr-tracker-bot clearly catches most cases as it is, and
it's only the occasional "somebody did something odd" that then misses
an automated response. Not a huge deal. For me it was actually more
the "I didn't understand why the response didn't happen", not so much
"I really want to always see responses".

                Linus

Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[Konstantin Ryabitsev]

On Mon, Aug 05, 2019 at 12:17:49PM -0700, Linus Torvalds wrote:
>> However, I suspect that getting message-ids for all your pull 
>> requests
>> would significantly complicate your workflow.
>
>Yeah, that would be a noticeable annoyance. If I were to process pull
>requests the way I used to process emailed patches (ie "git am -s" on
>a mailbox) that would be a natural thing to perhaps do, but it's not
>at all how it ends up working. Having to save the pull request email
>to then process it with some script would turn it into a chore.
>
>I think the pr-tracker-bot clearly catches most cases as it is, and
>it's only the occasional "somebody did something odd" that then misses
>an automated response. Not a huge deal. For me it was actually more
>the "I didn't understand why the response didn't happen", not so much
>"I really want to always see responses".

Ok, let me add a fix for Re: at the start -- this won't make things
significantly more expensive, but will catch this particular corner 
case.

Best regards,
-K

Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[Micah Morton]

On Mon, Aug 5, 2019 at 12:27 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
>
> On Mon, Aug 05, 2019 at 12:17:49PM -0700, Linus Torvalds wrote:
> >> However, I suspect that getting message-ids for all your pull
> >> requests
> >> would significantly complicate your workflow.
> >
> >Yeah, that would be a noticeable annoyance. If I were to process pull
> >requests the way I used to process emailed patches (ie "git am -s" on
> >a mailbox) that would be a natural thing to perhaps do, but it's not
> >at all how it ends up working. Having to save the pull request email
> >to then process it with some script would turn it into a chore.
> >
> >I think the pr-tracker-bot clearly catches most cases as it is, and
> >it's only the occasional "somebody did something odd" that then misses
> >an automated response. Not a huge deal. For me it was actually more
> >the "I didn't understand why the response didn't happen", not so much
> >"I really want to always see responses".
>
> Ok, let me add a fix for Re: at the start -- this won't make things
> significantly more expensive, but will catch this particular corner
> case.
>
> Best regards,
> -K

Linus, thanks for the tips earlier about gitk. I'll use that in the future.

Unfortunately I didn't have the mental model quite right of what
happens during the pull request. I was thinking along the lines of my
commits being cherry picked onto your tree, rather than how it
actually happens with git merge where my tree's commit history needs
to match yours perfectly.

Re: [GIT PULL] SafeSetID MAINTAINERS file update for v5.3

[Linus Torvalds]

On Tue, Aug 6, 2019 at 9:32 AM Micah Morton <mortonm@chromium.org> wrote:
>
> Unfortunately I didn't have the mental model quite right of what
> happens during the pull request. I was thinking along the lines of my
> commits being cherry picked onto your tree, rather than how it
> actually happens with git merge where my tree's commit history needs
> to match yours perfectly.

The "cherry-pick" model is what "git pull --rebase" does in reverse
(ie it pulls the exact history from the other end, and then rebases
the _local_ history on top of that).

But the cherry-picking model is entirely inappropriate for any bigger
project. Yes, you can do it locally on your _local_ small changes (but
see all the docs about why rebasing is not a great thing), but it's
entirely unmanageable and doesn't scale in the big picture.

It's why all the projects that were based on patch series were
complete and utter failures. A "patch series" only works locally. It's
not reasonable to scale and distribute.

Git fundamentally makes history a first-class immutable citizen, and
it's a major feature and a core design thing, and it's the _only_
thing that makes distribution possible. Whenever you rewrite history,
you fundamentally screw up a distributed model.

             Linus