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
