On 10/18/17 11:30, Rob Herring wrote: > On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou > <pantelis.antoniou@xxxxxxxxxxxx> wrote: >> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote: >>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull <atull@xxxxxxxxxx> wrote: >>>> On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand <frowand.list@xxxxxxxxx> wrote: >>>>> On 10/17/17 14:46, Rob Herring wrote: >>>>>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull <atull@xxxxxxxxxx> wrote: >>>>>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring <robh@xxxxxxxxxx> wrote: >>>>>>> >>>>>>> Hi Rob, >>>>>>> >>>>>>>> With dependencies on a statically allocated full path name converted to >>>>>>>> use %pOF format specifier, we can store just the basename of node, and >>>>>>>> the unflattening of the FDT can be simplified. >>>>>>>> >>>>>>>> This commit will affect the remaining users of full_name. After >>>>>>>> analyzing these users, the remaining cases should only change some print >>>>>>>> messages. The main users of full_name are providing a name for struct >>>>>>>> resource. The resource names shouldn't be important other than providing >>>>>>>> /proc/iomem names. >>>>>>>> >>>>>>>> We no longer distinguish between pre and post 0x10 dtb formats as either >>>>>>>> a full path or basename will work. However, less than 0x10 formats have >>>>>>>> been broken since the conversion to use libfdt (and no one has cared). >>>>>>>> The conversion of the unflattening code to be non-recursive also broke >>>>>>>> pre 0x10 formats as the populate_node function would return 0 in that >>>>>>>> case. >>>>>>>> >>>>>>>> Signed-off-by: Rob Herring <robh@xxxxxxxxxx> >>>>>>>> --- >>>>>>>> v2: >>>>>>>> - rebase to linux-next >>>>>>>> >>>>>>>> drivers/of/fdt.c | 69 +++++++++----------------------------------------------- >>>>>>>> 1 file changed, 11 insertions(+), 58 deletions(-) >>>>>>> >>>>>>> I've just updated to the latest next branch and am finding problems >>>>>>> applying overlays. Reverting this commit alleviates things. The >>>>>>> errors I get are: >>>>>>> >>>>>>> [ 88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0 >>>>>>> [ 88.513447] OF: overlay: apply failed '/__symbols__' >>>>>>> [ 88.518423] create_overlay: Failed to create overlay (err=-12) >>>>>> >>>>>> Frank's series with overlay updates should fix this. >>>>> >>>>> Yes, it does: >>>>> >>>>> [PATCH v3 11/12] of: overlay: remove a dependency on device node full_name >>>> >>>> Thanks for the fast response. I fetched the dt/next branch to test >>>> this but there are sufficient changes that Pantelis' "OF: DT-Overlay >>>> configfs interface (v7)" is broken now. I've been adding that >>>> downstream since 4.4. We're using it as an interface for applying >>>> overlays to program FPGAs. If we fix it again, is there any chance >>>> that can go upstream now? >>> >>> With a drive-by posting once every few years, no. >>> >> >> I take offense to that. There's nothing changed in the patch for years. >> Reposting the same patch without changes would achieve nothing. > > Are you still expecting review comments on it or something? > Furthermore, If something is posted infrequently, then I'm not > inclined to comment or care if the next posting is going to be after I > forget what I previously said (which is not very long). > > I'm just saying, don't expect to forward port, post and it will be > accepted. Below is minimally one of the issues that needs to be > addressed. > >>> The issue remains that the kernel is not really setup to deal with any >>> random property or node to be changed at any point in run-time. I >>> think there needs to be some restrictions around what the overlays can >>> touch. We can't have it be wide open and then lock things down later >>> and break users. That paragraph is key to any discussion of accepting code to apply overlays. Solving that issue has been stated to be a gating factor for such code from the beginning of overlay development. >>> One example of what you could do is you can only add >>> sub-trees to whitelisted nodes. That's probably acceptable for your >>> usecase. >>> >> >> Defining what can and what cannot be changed is not as trivial as a >> list of white-listed nodes. > > No, but we have to start somewhere and we are not starting with any > change allowed anywhere at anytime. If that is what people want, then > they are going to get to maintain that out of tree. > >> In some cases there is a whole node hierarchy being inserted (like in >> a FPGA). > > Yes, so you'd have a target fpga region. That sounds fine to me. Maybe > its not a static whitelist, but drivers have to register target > nodes/paths. > >> In others, it's merely changing a status property to "okay" and >> a few device parameters. > > That seems fine too. Disabled nodes could be allowed. But what if you > add/change properties on a node that is not disabled? Once a node is > enabled, who is responsible for registering the device? > > What about changing a node from enabled to disabled? The kernel would > need to handle that or not allow it. > >> The real issue is that the kernel has no way to verify that a given >> device tree, either at boot time or at overlay application time, is >> correct. >> >> When the tree is wrong at boot-time you'll hang (if you're lucky). >> If the tree is wrong at run-time you'll get some into some unidentified >> funky state. > > Or have some security hole or a mechanism for userspace to crash the system. > >> Finally what is, and what is not 'correct' is not for the kernel to >> decide arbitrarily, it's a matter of policy, different for each >> use-case. > > It is if the kernel will break doing so. > > Rob > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
