Re: [PATCH v3] of: overlay: user space synchronization

[Rob Herring]

On Thu, Oct 18, 2018 at 7:06 PM Frank Rowand <frowand.list@xxxxxxxxx> wrote:
>
> On 10/18/18 12:32, Rob Herring wrote:
> > On Tue, Oct 16, 2018 at 05:34:26PM -0700, frowand.list@xxxxxxxxx wrote:
> >> From: Frank Rowand <frank.rowand@xxxxxxxx>
> >>
> >> When an overlay is applied or removed, the live devicetree visible in
> >> /proc/device-tree/, aka /sys/firmware/devicetree/base/, reflects the
> >> changes.  There is no method for user space to determine whether the
> >> live devicetree was modified by overlay actions.
> >
> > Because userspace has no way to modify the DT and the ways the kernel
> > can do modifications is limited.
> >
> > Do you have an actually need for this outside of testing/development?
>
> I do not know if anyone uses /proc/device-tree for anything outside of
> testing/development.  If we believe that there is no other use of
> /proc/device-tree we can simply document that there is no expectation
> that accessors will see a consistent, unchanging /proc/device-tree.

I didn't mean whether /proc/device-tree is used outside of testing. It
is. The question is whether any users care if there are changes
happening. If so what is the use case?

kexec used to be one of the main users, but I think it has switched
over to the exported FDT which matches what the kernel was originally
passed.

>
> That would be a much smaller patch.
>
>
> >> Provide a sysfs file, /sys/firmware/devicetree/tree_version,  to allow
> >> user space to determine if the live devicetree has remained unchanged
> >> while a series of one or more accesses of /proc/device-tree/ occur.
> >>
> >> The use of both (1) dynamic devicetree modifications and (2) overlay
> >> apply and removal are not supported during the same boot cycle.  Thus
> >> non-overlay dynamic modifications are not reflected in the value of
> >> tree_version.
> >
> > I'd prefer to see some sort of information on overlays exported and user
> > space can check if that changed. IIRC, Pantelis had a series to do that
> > along with a kill switch to prevent further modifications. At least some
> > of that series only had minor issues to fix.
>
> The kill switch addresses a different concern, which was from the security
> community.  The kill switch is on my todo list.

Yes, but there could be other uses. It's not a big step from wanting
to know if the DT has changed to wanting to control it changing or
not.

Perhaps the kill switch needs 2 levels: a temporary freeze and a
permanent freeze. In any case, they don't seem completely unrelated
and I don't really want to see userspace ABI added bit by bit.

> I don't remember exactly what info the overlay information export patch
> provided.  I'll have to go find it and re-read it.
>
>
> > Also, shouldn't we get uevents if the tree changes? Maybe that's not
>
> Yes (off the top of my head).  But a shell script accessing /proc/device-tree
> is not going to get uevents.

No, but userspace can get them. Accessible from a shell script is not
a requirement of kernel interfaces.

Rob