linux-kernel - Re: [PATCH 2/2] gpiolib: cdev: export the consumer's PID

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: <CAMRc=Me=QxXRgZKyirj23r4hEN9bzcPSM6N4z=0yGgAZheh=Qg@mail.gmail.com>
Date:   Mon, 12 Sep 2022 11:56:17 +0200
From:   Bartosz Golaszewski <brgl@...ev.pl>
To:     Kent Gibson <warthog618@...il.com>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] gpiolib: cdev: export the consumer's PID

On Mon, Sep 12, 2022 at 11:53 AM Kent Gibson <warthog618@...il.com> wrote:
>

[snip]

> > >
> > > My knee-jerk reaction here was to make the pid unsigned, as we never
> > > pass a negative PID.
> > > Keeping in mind that the existing kernel will return 0 for this field
> > > (the existing padding), so 0 needs to be excluded from valid PIDs
> > > anyway.
> > >
> > > Andy suggests returning -1 for kernel held lines.
> > > In that case 0 would mean "old kernel", while -1 would mean "kernel
> > > held".
> > >
> > > As libgpiod will have to convert the 0 to -1 when returning the PID to
> > > user-space as a pid_t, I'm good with the uAPI using 0 to mean
> > > "no PID available" for all cases. I'm still open to passing -1 for
> > > kernel held is there is a use case for it, but I don't see one.
> > >
> >
> > Using -1 sounds good but I've just realized there's a different
> > problem. A process holding a file descriptor may fork and both the
> > parent and the child will keep the same file descriptors open. Now
> > we'll have two processes (with different PIDs) holding the same GPIO
> > lines (specifically holding a file descriptor to the same anonymous
> > inode).
> >
> > This already poses a problem for this patch as we'd need to return an
> > array of PIDs which we don't have the space for but also is a
> > situation which we haven't discussed previously IIRC - two processes
> > keeping the same GPIO lines requested.
> >
> > I don't have any good idea on how to address this yet. One thing off
> > the top of my head is: close the parent's file descriptor from kernel
> > space (is it even possible?) on fork() (kind of like the close() on
> > exec flag).
> >
> > I need to think about it more.
> >
>
> I thought the O_CLOEXEC was set on the request fds exactly to prevent this
> case - only one process can hold the request fd.
>

O_CLOEXEC means "close on exec" not "close on fork". When you fork,
you inherit all file descriptors from your parent. Only once you call
execve() are the fds with this flag closed *in the child*.

Bart