linux-kernel - RE:(2) (2) [PATCH v2] driver/nvme/host: Support duplicated nsid for the private ns

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: <20220315105630epcms2p48f825f28577c2cd816c06c0f47bd3cc9@epcms2p4>
Date:   Tue, 15 Mar 2022 19:56:30 +0900
From:   Sungup Moon <sungup.moon@...sung.com>
To:     "hch@....de" <hch@....de>
CC:     "kbusch@...nel.org" <kbusch@...nel.org>,
        "axboe@...com" <axboe@...com>,
        "sagi@...mberg.me" <sagi@...mberg.me>,
        "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE:(2) (2) [PATCH v2] driver/nvme/host: Support duplicated nsid for
 the private ns

> 
> On Tue, Mar 15, 2022 at 06:56:12PM +0900, Sungup Moon wrote:
> > I'll answer your opinion.
> > 
> > 1. ns_head leak issue
> > 
> > I don't think that is leaked ns_head. Because although all ids and nsid are same
> > through all namespaces, each namespaces are indenpendent namespace and each of that
> > should have independent data structure.
> > Duplicated nsid private namespace is different from the shared namespace even though
> > same information.
>  
> In your patch if nvme_find_ns_head returns a head for a private namespace
> with a non-uniqueue ID, that returned head already has an additional
> reference, which we'll never drop as the head variable is overriden just
> below.

Ok, I will review my patch with my team and reply your comment asap.
  
> > 2. nvme_mpath_set_disk_name issue
> > 
> > Yes, I also agree that subsystem-wide IDA is very important data. However, I
> > implemented without nvme_mpath_set_disk_name modification at the first time, it is
> > hard to decide which namespace are connected to private controller.
>  
> It is not just very important, it can't work without that.  The two
> separate IDAs can will have conflicts, so you can up with two namespaces
> with the same name without that change.
>  
> > As you know, each nvme controller start initiating at a time. So, each controller
> > structures are sequentially initiated, but each namespaces structures are initiated
> > independently because of multi-processing on cpu. So, all namespace can have different
> > instance number every boot-up time, and it makes hard to track and control the private
> > namespace on the controller or device failure.
>  
> Yes, but that is true of all Linux device enumeration.  That is why
> everyone should use table identifiers like the UUID or GUID to identify
> the namespaces.
>  
> > Anyway, the private namespace is same condition with no-multipath situation (because
> > private namespace cannot shared between controllers) so I think that the private
> > namespace should follow the naming rule with no-multipath situation.
>  
> We can't use the non-multipath cabale naming as it will cause conflicts
> in the naming.  If anything in the system supports multipathing we have
> to use subsystem based instances for the naming.
>  

Ok, I agree your opinion. My patch about nvme_mpath_set_disk_name will make confusion
of the naming in subsystem, and also find the relation-ship between controller <->
namespace using the sysfs directory structure. I'll remove that patch line and resummit
with first issue (ns_head leak issue) after review that.