Re: [PATCH 3/4] software node: verify that property data is not on stack

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Wed, Mar 25, 2026 at 01:51:02PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 24, 2026 at 09:17:18AM -0700, Dmitry Torokhov wrote:
> > On Tue, Mar 24, 2026 at 02:33:31PM +0200, Andy Shevchenko wrote:
> > > On Mon, Mar 23, 2026 at 05:39:39PM -0700, Dmitry Torokhov wrote:
> > > > When registering a software node, ensure that the property data is not
> > > > located on the stack, as it is expected to persist for the lifetime of
> > > > the node.
> 
> ...
> 
> > > > +#include <linux/sched/task_stack.h>
> > > 
> > > Looking at this name the use of it here doesn't sound right...
> > 
> > Because ... ? object_is_on_stack() is defined there.
> 
> Because it's defined there. The key word here I see is 'task' in the name of
> the header, without Ack from sched folks, I am not convinced we can use that at
> any point in the kernel.

It is fine to be used in drivers, and it is used in USB
(drivers/usb/core/hcd.c), SPI (drivers/spi/spi-mem.c) and others.

"task" is not magical, it simply refers to a process or a thread (user
or kernel). This particular macro just checks if the object is within
limits of stack area of currently executing thread.

> 
> ...
> 
> > > > +	for (prop = node->properties; prop && prop->name; prop++) {
> > > > +		if (!prop->is_inline && object_is_on_stack(prop->pointer)) {
> > > > +			pr_err("%s: property data can't be on stack\n", __func__);
> > > > +			return -EINVAL;
> > > > +		}
> > > > +	}
> > > 
> > > And again same question, why we can't simply dup them as we do for (string)
> > > arrays?
> > 
> > Because majority of users of software_node_register() deal with static
> > const properties so duping them is waste of memory.
> 
> So, then why can't we do that for the references? Just then copy and free at
> software unregistration if required.

We do not want to copy and free because that would be waste of memory in
majority of cases where software_node_register() is used.

When dealing with other kinds of properties it is pretty obvious where
the objectr itself lives, but when using PROPERTY_ENTRY_REF() and
PROPERTY_ENTYR_GPIO() it is very easy to miss (I did! and I made them)
that the reference args object is on stack as it is hidden behind a
macro.

So it is just a safeguard warning us ahead of time instead of needing to
figure out why a driver can't find GPIO even through the reference looks
right.

Thanks.

-- 
Dmitry