netdev - Re: [PATCH 2/5] dccp: Auto-load (when supported) CCID plugins for negotiation

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]

Date:	Sun, 14 Dec 2008 15:50:30 +0100
From:	"Michał Mirosław" <mirqus@...il.com>
To:	"Gerrit Renker" <gerrit@....abdn.ac.uk>
Cc:	davem@...emloft.net, dccp@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH 2/5] dccp: Auto-load (when supported) CCID plugins for negotiation

2008/12/13 Gerrit Renker <gerrit@....abdn.ac.uk>:
> | > +static int ccid_request_module(u8 id)
> | > +{
> | > +       if (!in_atomic()) {
> | > +               ccids_read_lock();
> | > +               if (ccids[id] == NULL) {
> | > +                       ccids_read_unlock();
> | > +                       return request_module("net-dccp-ccid-%d", id);
> | > +               }
> | > +               ccids_read_unlock();
> | > +       }
> | > +       return 0;
> | > +}
> |
> | Just a random thought: does this lock really do anything useful here?
> |
> Reading the (shared) 'ccids' array is the solution chosen to check whether
> the module for CCID with number 'id' is already loaded.
>
> It would be bad to call request_module() each time a new DCCP socket is
> opened. Using the 'ccids' array may not be the only way to check whether
> a given module (whose name depends on the value of 'id') is loaded.
>
> But if this solution is chosen, then it requires to protect the read
> access to 'ccids', which is shared among all DCCP sockets.

Since the lock is dropped after checking ccids[id] then there's
a window where multiple request_module()s can be called if multiple
applications create a DCCP socket at a same time. The code below
should do the same without a lock (ccids is a static array,
so ccids[N] is always at the same place).

static int ccid_request_module(u8 id)
{
       if (!in_atomic()) {
               rmb();
               if (ccids[id] == NULL)
                       return request_module("net-dccp-ccid-%d", id);
       }
       return 0;
}

Best Regards,
Michał Mirosław