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]
Message-ID: <20180710135813.65ad0d7b@cakuba.lan>
Date:   Tue, 10 Jul 2018 13:58:13 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Andrey Ignatov <rdna@...com>, alexei.starovoitov@...il.com,
        oss-drivers@...ronome.com, netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next v2 11/12] tools: libbpf: allow map reuse

On Tue, 10 Jul 2018 20:16:46 +0200, Daniel Borkmann wrote:
> On 07/10/2018 08:09 PM, Jakub Kicinski wrote:
> > On Mon, 9 Jul 2018 21:23:20 -0700, Andrey Ignatov wrote:  
> >> Jakub Kicinski <jakub.kicinski@...ronome.com> [Mon, 2018-07-09 19:49 -0700]:  
> >>> On Mon, 9 Jul 2018 13:22:54 -0700, Andrey Ignatov wrote:    
> >>>> Jakub Kicinski <jakub.kicinski@...ronome.com> [Mon, 2018-07-09 11:01 -0700]:  
> >>>>> +	map->fd = dup(fd);      
> >>>>
> >>>> Unfortunately, new descriptor created by dup(2) will not have O_CLOEXEC set, in
> >>>> contrast to original fd returned by kernel on map creation.
> >>>>
> >>>> libbpf has other interface shortcomings where it comes up. E.g. struct
> >>>> bpf_object owns all descriptors it contains (progs, maps) and closes them in
> >>>> bpf_object__close(). if one wants to open/load ELF, then close it but
> >>>> keep, say, prog fd to attach it to cgroup some time later, then fd
> >>>> should be duplicated as well to get a new one not owned by bpf_object.
> >>>>
> >>>> Currently I use this workaround to avoid time when new fd doesn't have
> >>>> O_CLOEXEC:
> >>>>
> >>>> 	int new_prog_fd = open("/dev/null", O_RDONLY | O_CLOEXEC);
> >>>> 	if (new_prog_fd < 0 ||
> >>>> 	    dup3(bpf_program__fd(prog), new_prog_fd, O_CLOEXEC) == -1) {
> >>>> 		/* .. handle error .. */
> >>>> 		close(new_prog_fd);
> >>>> 	}
> >>>> 	/* .. use new_prog_fd with O_CLOEXEC set */
> >>>>
> >>>> Not sure how to simplify it. dup2() has same problem with regard to
> >>>> O_CLOEXEC.
> >>>>
> >>>> Use-case: standalone server application that uses libbpf and does
> >>>> fork()/execve() a lot.    
> >>>
> >>> Good point!  I have no better ideas.  Although being slightly paranoid
> >>> I would perhaps use "/" instead of "/dev/null"?  Shouldn't matter?    
> >>
> >> No strong preferences, important thing is to create fd with O_CLOEXEC
> >> set somehow.
> >>
> >> Is it safer to use "/" than "/dev/null"? (trying to understand if I
> >> should change my code as well)  
> > 
> > IDK :)  Could there be a crazy scenario when someone runs chroot or a
> > very broken system without /dev/null?  / should always be there?
> > 
> > Thanks for all the other reviews, I will update the code accordingly!  

Oh no, dup3() is under _GNU_SOURCE as well.  libbpf kind of depends
on XSI-compliant strerror_r() semantics, I think I'll have to move the
strerror_r()-dependent code out of the libbpf.c file :(

> Ok, I've tossed the v2 series from patchwork and waiting for your respin
> in that case.

Yes, thank you!

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ