Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210531153410.93150-1-changbin.du@gmail.com> <20210531220128.26c0cb36@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
 <CAM_iQpUEjBDK44=mD5shkmmoDYhmHQaSZtR34rLRkgd9wSWiQQ@mail.gmail.com> <20210602091451.kbdul6nhobilwqvi@wittgenstein>
In-Reply-To: <20210602091451.kbdul6nhobilwqvi@wittgenstein>
From:   Cong Wang <xiyou.wangcong@gmail.com>
Date:   Thu, 3 Jun 2021 15:52:29 -0700
Message-ID: <CAM_iQpUqgeoY_mA6cazUPCWwMK6yw9SaD6DRg-Ja4r6r_zOmLg@mail.gmail.com>
Subject: Re: [PATCH] nsfs: fix oops when ns->ops is not provided
To:     Christian Brauner <christian.brauner@ubuntu.com>
Cc:     Jakub Kicinski <kuba@kernel.org>,
        Changbin Du <changbin.du@gmail.com>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        "David S. Miller" <davem@davemloft.net>,
        Linux Kernel Network Developers <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        stable <stable@vger.kernel.org>,
        David Laight <David.Laight@aculab.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Jun 2, 2021 at 2:14 AM Christian Brauner
<christian.brauner@ubuntu.com> wrote:
> But the point is that ns->ops should never be accessed when that
> namespace type is disabled. Or in other words, the bug is that something
> in netns makes use of namespace features when they are disabled. If we
> handle ->ops being NULL we might be tapering over a real bug somewhere.

It is merely a protocol between fs/nsfs.c and other namespace users,
so there is certainly no right or wrong here, the only question is which
one is better.

>
> Jakub's proposal in the other mail makes sense and falls in line with
> how the rest of the netns getters are implemented. For example
> get_net_ns_fd_fd():

It does not make any sense to me. get_net_ns() merely increases
the netns refcount, which is certainly fine for init_net too, no matter
CONFIG_NET_NS is enabled or disabled. Returning EOPNOTSUPP
there is literally saying we do not support increasing init_net refcount,
which is of course false.

> struct net *get_net_ns_by_fd(int fd)
> {
>         return ERR_PTR(-EINVAL);
> }

There is a huge difference between just increasing netns refcount
and retrieving it by fd, right? I have no idea why you bring this up,
calling them getters is missing their difference.

Thanks.