Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190606085524.GA21119@zhanggen-UX430UQ> <CAFqZXNvM94T2reUsn6Mwuz6GNGNCR=wUNBE8w4tcjNuhJ6rCeQ@mail.gmail.com>
In-Reply-To: <CAFqZXNvM94T2reUsn6Mwuz6GNGNCR=wUNBE8w4tcjNuhJ6rCeQ@mail.gmail.com>
From:   Paul Moore <paul@paul-moore.com>
Date:   Mon, 10 Jun 2019 16:20:28 -0400
Message-ID: <CAHC9VhT+e2Z+4=5P0g4B4F1g0w2SkQjwUnhQkmu5V+HvuZi8Cg@mail.gmail.com>
Subject: Re: [PATCH v4] selinux: lsm: fix a missing-check bug in
 selinux_sb_eat_lsm_o pts()
To:     Ondrej Mosnacek <omosnace@redhat.com>,
        Gen Zhang <blackgod016574@gmail.com>
Cc:     Stephen Smalley <sds@tycho.nsa.gov>,
        Eric Paris <eparis@parisplace.org>, selinux@vger.kernel.org,
        Linux kernel mailing list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jun 7, 2019 at 4:41 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> On Thu, Jun 6, 2019 at 10:55 AM Gen Zhang <blackgod016574@gmail.com> wrote:
> > In selinux_sb_eat_lsm_opts(), 'arg' is allocated by kmemdup_nul(). It
> > returns NULL when fails. So 'arg' should be checked. And 'mnt_opts'
> > should be freed when error.
> >
> > Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
> > Fixes: 99dbbb593fe6 ("selinux: rewrite selinux_sb_eat_lsm_opts()")
>
> My comments about the subject and an empty line before label apply
> here as well, but Paul can fix both easily when applying ...

Since we've been discussing general best practices for submitting
patches in this thread (and the other related thread), I wanted to
(re)clarify my thoughts around maintainers fixing patches when merging
them upstream.

When in doubt, do not ever rely on the upstream maintainer fixing your
patch while merging it, and if problems do arise during review, it is
best to not ask the maintainer to fix them for you, but for you to fix
them instead (you are the patch author after all!).  Similarly, making
comments along the lines of "X can fix both easily when applying", is
also a bad thing to say when reviewing patches.  It's the patch
author's responsibility to fix the patch by address review comments,
not the maintainer.  I'll typically let you know if you don't need to
rework a patch(set).

That said, there are times when the maintainer will change the patch
during merging, most of which are due to resolving merge
conflicts/fuzz with changes already in the tree (that *is* the
maintainer's responsibility).  Speaking for myself, sometimes I will
also make some minor changes if the patch author is away, or
unreliable, or if there is a hard deadline near and I'm worried that
the updated patch might not be ready in time.  I'll also sometimes
make the changes directly if the patch is holding up a larger, more
important patch(set), but that is really rare.  I'm sure I've made
changes for other reasons in the past, and I'm sure I'll make changes
for other reasons in the future, but hopefully this will give you a
better idea of how the process works :)

-- 
paul moore
www.paul-moore.com