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

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 6/5/25 14:03, 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.
You could have a preliminary commit before this one that removes a 
clearly vulnerable code, and then this commit can move it. Or reverse it 
and do the code move and fix it.
> While I agree it should probably be a switch statement (or factored away
> altogether), this isn't the place for it.
All due respect, but leaving code that is clearly a sitting 
vulnerability mine is not really a great choice.
V/r,
dps