linux-kernel - Re: [PATCH v4] selinux: lsm: fix a missing-check bug in selinux_sb_eat_lsm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 10 Jun 2019 20:05:31 -0700
From:   Gen Zhang <blackgod016574@...il.com>
To:     Paul Moore <paul@...l-moore.com>
Cc:     Ondrej Mosnacek <omosnace@...hat.com>,
        Stephen Smalley <sds@...ho.nsa.gov>,
        Eric Paris <eparis@...isplace.org>, selinux@...r.kernel.org,
        Linux kernel mailing list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4] selinux: lsm: fix a missing-check bug in
 selinux_sb_eat_lsm_o pts()

On Mon, Jun 10, 2019 at 04:20:28PM -0400, Paul Moore wrote:
> On Fri, Jun 7, 2019 at 4:41 AM Ondrej Mosnacek <omosnace@...hat.com> wrote:
> >
> > On Thu, Jun 6, 2019 at 10:55 AM Gen Zhang <blackgod016574@...il.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@...il.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
Thanks for your comments. I will resend a patch after revising.

Thanks
Gen