netdev - Re: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZfgI2Aow6751-EGj@framework>
Date: Mon, 18 Mar 2024 10:26:48 +0100
From: Max Gautier <mg@....gautier.name>
To: Ratheesh Kannoth <rkannoth@...vell.com>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create
 /var/lib/arpd on first use

On Mon, Mar 18, 2024 at 09:18:59AM +0000, Ratheesh Kannoth wrote:
> > > > > > +	if (strcmp(default_dbname, dbname) == 0
> > > > > > +			&& mkdir(ARPDDIR, 0755) != 0
> > > > > > +			&& errno != EEXIST
> > > > > why do you need errno != EEXIST case ? mkdir() will return error
> > > > > in this case
> > > > as well.
> > > >
> > > > EEXIST is not an error in this case: if the default location already
> > > > exist, all is good. mkdir would still return -1 in this case, so we
> > > > need to exclude it manually.
> > >
> > > ACK. IMO, it would make a more readable code if you consider splitting the
> > "if" loop.
> > 
> > Something like this ? I tend to pack conditions unless branching is necessary,
> > but no problem if this form is preferred.
> > 
> > if (strcmp(default_dbname, dbname) == 0) {
> >     if (mkdir(ARPDDIR, 0755) != 0 && errno != EEXIST) {
> >    ...
> >    }
> > }
> ACK.   
> instead of errno != EXIST ,  you may consider stat() before mkdir() call. Just my way thinking(please ignore it, if you don't like). 
> My thinking is --> you need to execute mkdir () only first time, second time onwards, stat() call will return 0.

That's racy: we can stat and have a non existing folder, then have
another arpd instance (or anything else, really) create the directory,
and we would hit EEXIST anyway when we call mkdir.
Also, that needs two syscalls instead of one.

-- 
Max Gautier