On Mon, Nov 25, 2024 at 01:33:15PM +0200, Tomi Valkeinen wrote: > On 25/11/2024 10:39, Sakari Ailus wrote: > > On Fri, Nov 22, 2024 at 04:37:12PM +0200, Cosmin Tanislav wrote: > >> When using v4l2_subdev_set_routing to set a subdev's routing, and the > >> passed routing.num_routes is 0, kmemdup is not called to populate the > >> routes of the new routing (which is fine, since we wouldn't want to pass > >> a possible NULL value to kmemdup). > >> > >> This results in subdev's routing.routes to be NULL. > >> > >> routing.routes is further used in some places without being guarded by > >> the same num_routes non-zero condition. > >> > >> Fix it. > > > > While I think moving the code to copy the routing table seems reasonable, > > is there a need to make num_routes == 0 a special case? No memcpy() > > implementation should access destination or source if the size is 0. > > I think so too, but Cosmin convinced me that the spec says otherwise. > > From the C spec I have, in "7.21.1 String function conventions": > > " > Where an argument declared as size_t n specifies the length of the array for a > function, n can have the value zero on a call to that function. Unless explicitly stated > otherwise in the description of a particular function in this subclause, pointer arguments > on such a call shall still have valid values, as described in 7.1.4. > " > > The memcpy section has no explicit mention that would hint otherwise. > > In 7.1.4 Use of library functions it says that unless explicitly stated > otherwise, a null pointer is an invalid value. > > That said, I would still consider memcpy() with size 0 always ok, > regardless of the src or dst, as the only memcpy implementation we need > to care about is the kernel's. I was going to mention that too. The kernel C library API is modeled on the standard C library API, but it takes quite a few liberties. What I think is important in the context of this patch is to ensure consistency in how we model our invariants. I'm less concerned about relying on memcpy() being a no-op that doesn't dereference pointers when the size is 0 (provided the caller doesn't otherwise trigger C undefined behaviours) than about the consistency in how we model routing tables with no entry. I'd like to make sure that num_routes == 0 always implies routes == NULL and vice versa (which may already be the case, I haven't checked). -- Regards, Laurent Pinchart
