netdev - Re: [PATCH] RFC: ipv4: share sysctl net/ipv4/conf/DEVNAME/ tables

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]

Date:	Fri, 28 Jan 2011 08:21:52 +0200
From:	Lucian Adrian Grijincu <lucian.grijincu@...il.com>
To:	Alexey Dobriyan <adobriyan@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH] RFC: ipv4: share sysctl net/ipv4/conf/DEVNAME/ tables

[Resent to the list because the last reply got rejected because of HTML]


On Sat, Jan 15, 2011 at 12:41 PM, Alexey Dobriyan <adobriyan@...il.com> wrote:
> I wonder where interactions with device renaming are handled.

I did some digging and I'm pretty confident that this will not cause
problems with regards to device renaming.


On device rename these are the relevant call stacks:


RENAME
• dev_ioctl
  ∘ rtnl_lock();
  ∘ dev_ifsioc(net, &ifr, cmd);
    ‣ dev_change_name
      • dev_get_valid_name
        ∘ strlcpy(dev->name, name, IFNAMSIZ)
      • call_netdevice_notifiers(NETDEV_CHANGENAME);
        ∘ inetdev_event
          ‣ devinet_sysctl_unregister(in_dev)
            • unregister_sysctl_table(header)
              ∘ lock sysctl
              ∘ start_unregistering(header);
                ‣ if (header->used) { unlock sysctl,
wait_for_completion; lock sysctl }
              ∘ unlock sysctl
          ‣ devinet_sysctl_register(in_dev)
  ∘ rtnl_unlock();




HANDLER
• proc_sys_call_handler
  ∘ head = grab_header(inode)
    ‣ sysctl_head_grab
      • lock sysctl
      • head->used++
      • unlock sysctl
  ∘ if (IS_ERR(head)) return err
  ∘ devinet_conf_handler
    ‣ dev_get_by_name(dev, filp->f_path.dentry->d_parent->d_name.name)
  ∘ sysctl_head_finish(head)
    ‣ lock sysctl
    ‣ if (--head->used && unregistering) complete()
    ‣ unlock sysctl


Compressed:
RENAME (under rtnl lock)
• R1: memcpy(dev->name, newname)
• R2: if the sysctl header is used wait until it's not used any more,
      mark header as invalid

HANDLER:
• H1: get header, if header invalid, return error
• H2: dev_get_by_name
• H3: if there's someone waiting to unregister, complete it's action


Only one rename can be in progress at a time (because of the
rtnl_lock), so cases like A->B, C->A cannot run in parallel.  To
finish a device rename, we need to unregister the sysctl table header
first.


• R2 < H1: a RENAME runs before a HANDLER, then the HANDLER
           will fail at H1 (the sysctl header will be made invalid at R2).
• H1 < R2:
  ∘ HANDLER acquired the header
    ‣ R1 < H2: dev_get_by_name will not find the device (because R1 renamed it)
    ‣ R1 > H2: dev_get_by_name will return the correct device
      	       (the name is still valid)



In conclusion, I don't see any race conditions and I don't see how we
could get the wrong device after a rename.

I've posted a new version of the patch with some improvements.


-- 
 .
..: Lucian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html