[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 12/19] xen/dt: Move bootfdt functions to xen/bootfdt.h



On Thu, 5 Jun 2025, Alejandro Vallejo wrote:
> On Mon Jun 2, 2025 at 10:25 PM CEST, Daniel P. Smith wrote:
> >> +/* Helper to read a big number; size is in cells (not bytes) */
> >> +static inline u64 dt_read_number(const __be32 *cell, int size)
> >> +{
> >> +    u64 r = 0;
> >> +
> >> +    while ( size-- )
> >> +        r = (r << 32) | be32_to_cpu(*(cell++));
> >> +    return r;
> >> +}
> >
> > I know you are trying to keep code changes to a minimal but let's not 
> > allow poorly constructed logic like this to continue to persist. This is 
> > an unbounded, arbitrary read function that is feed parameters via 
> > externally input. The DT spec declares only two number types for a 
> > property, u32 and u64, see Table 2.3 in Section 2.2.4. There is no 
> > reason to have an unbounded, arbitrary read function lying around 
> > waiting to be leveraged in exploitation.
> 
> Seeing how it's a big lump of code motion, I really don't want to play games
> or I will myself lose track of what I changed and what I didn't.
> 
> While I agree it should probably be a switch statement (or factored away
> altogether), this isn't the place for it.

The improvement suggested by Daniel should be in a separate patch from
the code movement to make it easier to review, but it can still be part
of the patch series.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.