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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: 
 <MWHPR1801MB19182C5B4649293D29098689D32D2@MWHPR1801MB1918.namprd18.prod.outlook.com>
Date: Mon, 18 Mar 2024 09:43:52 +0000
From: Ratheesh Kannoth <rkannoth@...vell.com>
To: Max Gautier <mg@....gautier.name>
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

> From: Max Gautier <mg@....gautier.name>
> Sent: Monday, March 18, 2024 2:57 PM
> To: Ratheesh Kannoth <rkannoth@...vell.com>
> Cc: 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.
ACK. 

> Also, that needs two syscalls instead of one.
> 
> --
> Max Gautier