netdev - Re: rpc_xprt_debugfs_register() - atomic_inc

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <020aee05c808b3725db5679967406a918840f86f.camel@hammerspace.com>
Date:   Thu, 21 Jan 2021 17:56:26 +0000
From:   Trond Myklebust <trondmy@...merspace.com>
To:     "skhan@...uxfoundation.org" <skhan@...uxfoundation.org>,
        "Anna.Schumaker@...app.com" <Anna.Schumaker@...app.com>
CC:     "bfields@...ldses.org" <bfields@...ldses.org>,
        "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "chuck.lever@...cle.com" <chuck.lever@...cle.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: rpc_xprt_debugfs_register() - atomic_inc_return() usage

On Wed, 2021-01-20 at 16:52 -0700, Shuah Khan wrote:
> Hi Anna and Trond,
> 
> I came across the following while reviewing atomic_inc_return()
> usages
> that cast return value to unsigned
> 
> rpc_xprt_debugfs_register()'s atomic_inc_return() usage looks a bit
> odd.
> 
> - cur_id isn't initialized
> - id = (unsigned int)atomic_inc_return(&cur_id);
> 
> Please note that id is int. Is it expected that cur_id could
> overflow?
> Is there a maximum limit for this value?
> 

Yes, we do expect cur_id to eventually overflow (once you have created
2 billion RPC client instances), however the atomic increment
operations are expected to handle this correctly according to the
maintainers (I already asked them in a different context). Furthermore,
the code itself doesn't care about strict sequentiality. All it wants
from the counter is uniqueness, with that uniqueness condition actually
being enforced by the subsequent debugfs_create_file() call.

IOW: I don't think this is a real problem.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@...merspace.com