linux-kernel - Re: [PATCH v2] netfilter: xt_owner: Add supplementary groups option

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]

Message-ID: <0510351d1fbd17f11018b7c934fb3a525c265936.camel@samsung.com>
Date:   Thu, 09 May 2019 14:01:10 +0200
From:   Lukasz Pawelczyk <l.pawelczyk@...sung.com>
To:     Eric Dumazet <eric.dumazet@...il.com>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Jozsef Kadlecsik <kadlec@...ckhole.kfki.hu>,
        Florian Westphal <fw@...len.de>,
        "David S. Miller" <davem@...emloft.net>,
        netfilter-devel@...r.kernel.org, coreteam@...filter.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     Lukasz Pawelczyk <havner@...il.com>
Subject: Re: [PATCH v2] netfilter: xt_owner: Add supplementary groups option

On Thu, 2019-05-09 at 04:57 -0700, Eric Dumazet wrote:
> sk_socket keeps reference to f_cred. f_cred keeps reference to
> > group_info. As long as f_cred is alive and it doesn't seem to be
> > the
> > issue in the owner_mt() function, group_info should be alive as
> > well as
> > far as I can see. Its refcount will go down only when f_cred is
> > freed
> > (put_cred_rcu()).
> > 
> > If there is something I'm missing please correct me.
> 
> The problem is that you can´t clearly explain why the code is safe :/
> 
> Why would get_group_info() be needed then ?

Originally I though it wouldn't, that's why I did not include it in the
patch. Your question made me doubt that for a second. I also got
confused a little because the group_info code looked completely
different a while back, it got reworked and simplified.

> 
> You need to explain this in the changelog, so that future bug hunters
> do not have
> to guess.

Ok, I will.

-- 
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics