[PATCH 03/15] hw/alpha/dp264: Validate kernel and initrd sizes

Yodel Eldar posted 15 patches 4 weeks ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>
[PATCH 03/15] hw/alpha/dp264: Validate kernel and initrd sizes
Posted by Yodel Eldar 4 weeks ago
Add an underflow check when calculating the initrd base address.

Warn the user if initrd overlaps with kernel.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/dp264.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 27fbcee637..87af919895 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -61,7 +61,7 @@ static void clipper_init(MachineState *machine)
     long size, i;
     char *palcode_filename;
     uint64_t palcode_entry;
-    uint64_t kernel_entry, kernel_low;
+    uint64_t kernel_entry, kernel_low, kernel_high;
     unsigned int smp_cpus = machine->smp.cpus;
 
     /* Create up to 4 cpus.  */
@@ -165,7 +165,7 @@ static void clipper_init(MachineState *machine)
         uint64_t param_offset;
 
         size = load_elf(kernel_filename, NULL, cpu_alpha_superpage_to_phys,
-                        NULL, &kernel_entry, &kernel_low, NULL, NULL,
+                        NULL, &kernel_entry, &kernel_low, &kernel_high, NULL,
                         ELFDATA2LSB, EM_ALPHA, 0, 0);
         if (size < 0) {
             error_report("could not load kernel '%s'", kernel_filename);
@@ -181,7 +181,7 @@ static void clipper_init(MachineState *machine)
         }
 
         if (initrd_filename) {
-            long initrd_base;
+            hwaddr initrd_base;
             int64_t initrd_size;
 
             initrd_size = get_image_size(initrd_filename, NULL);
@@ -192,7 +192,15 @@ static void clipper_init(MachineState *machine)
             }
 
             /* Put the initrd image as high in memory as possible.  */
-            initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
+            if (usub64_overflow(ram_size, initrd_size, &initrd_base)) {
+                error_report("initial ram disk exceeds allotted ram size");
+                exit(1);
+            }
+            initrd_base &= TARGET_PAGE_MASK;
+            if (initrd_base <= kernel_high) {
+                warn_report("initial ram disk overlaps with kernel");
+            }
+
             load_image_targphys(initrd_filename, initrd_base,
                                 ram_size - initrd_base, NULL);
 

-- 
2.53.0
Re: [PATCH 03/15] hw/alpha/dp264: Validate kernel and initrd sizes
Posted by Richard Henderson 1 week, 6 days ago
On 3/11/26 08:31, Yodel Eldar wrote:
> @@ -192,7 +192,15 @@ static void clipper_init(MachineState *machine)
>               }
>   
>               /* Put the initrd image as high in memory as possible.  */
> -            initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
> +            if (usub64_overflow(ram_size, initrd_size, &initrd_base)) {
> +                error_report("initial ram disk exceeds allotted ram size");
> +                exit(1);
> +            }
> +            initrd_base &= TARGET_PAGE_MASK;
> +            if (initrd_base <= kernel_high) {
> +                warn_report("initial ram disk overlaps with kernel");
> +            }

Why is the first an error and the second a warning?


r~
Re: [PATCH 03/15] hw/alpha/dp264: Validate kernel and initrd sizes
Posted by Yodel Eldar 1 week, 6 days ago
Hi, Richard

On 25/03/2026 19:11, Richard Henderson wrote:
> On 3/11/26 08:31, Yodel Eldar wrote:
>> @@ -192,7 +192,15 @@ static void clipper_init(MachineState *machine)
>>               }
>>               /* Put the initrd image as high in memory as possible.  */
>> -            initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
>> +            if (usub64_overflow(ram_size, initrd_size, &initrd_base)) {
>> +                error_report("initial ram disk exceeds allotted ram 
>> size");
>> +                exit(1);
>> +            }
>> +            initrd_base &= TARGET_PAGE_MASK;
>> +            if (initrd_base <= kernel_high) {
>> +                warn_report("initial ram disk overlaps with kernel");
>> +            }
> 
> Why is the first an error and the second a warning?
> 

Initially, I had both as fatal, but then I figured a (mis)adventurous
user may actually want to experiment with kernel/initrd overlap, whereas
an underflow is something we can't allow (and initrd being larger than
RAM is physically impossible AFAIK). I could make the overlap fatal,
too, if that's better?

Thanks for the reviews in Patches 1-2, and for letting me know about
using cpu_to_le64 in Patch 4. v2 has seen some nontrivial changes in
the later patches, and I'll be sending it soon.

Thanks again,
Yodel

> 
> r~
> 


Re: [PATCH 03/15] hw/alpha/dp264: Validate kernel and initrd sizes
Posted by Richard Henderson 1 week, 6 days ago
On 3/26/26 12:04, Yodel Eldar wrote:
> Hi, Richard
> 
> On 25/03/2026 19:11, Richard Henderson wrote:
>> On 3/11/26 08:31, Yodel Eldar wrote:
>>> @@ -192,7 +192,15 @@ static void clipper_init(MachineState *machine)
>>>               }
>>>               /* Put the initrd image as high in memory as possible.  */
>>> -            initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
>>> +            if (usub64_overflow(ram_size, initrd_size, &initrd_base)) {
>>> +                error_report("initial ram disk exceeds allotted ram size");
>>> +                exit(1);
>>> +            }
>>> +            initrd_base &= TARGET_PAGE_MASK;
>>> +            if (initrd_base <= kernel_high) {
>>> +                warn_report("initial ram disk overlaps with kernel");
>>> +            }
>>
>> Why is the first an error and the second a warning?
>>
> 
> Initially, I had both as fatal, but then I figured a (mis)adventurous
> user may actually want to experiment with kernel/initrd overlap, whereas
> an underflow is something we can't allow (and initrd being larger than
> RAM is physically impossible AFAIK). I could make the overlap fatal,
> too, if that's better?

I can't see it being useful at all.


r~

Re: [PATCH 03/15] hw/alpha/dp264: Validate kernel and initrd sizes
Posted by Yodel Eldar 1 week, 5 days ago
On 25/03/2026 22:07, Richard Henderson wrote:
> On 3/26/26 12:04, Yodel Eldar wrote:
>> Hi, Richard
>>
>> On 25/03/2026 19:11, Richard Henderson wrote:
>>> On 3/11/26 08:31, Yodel Eldar wrote:
>>>> @@ -192,7 +192,15 @@ static void clipper_init(MachineState *machine)
>>>>               }
>>>>               /* Put the initrd image as high in memory as 
>>>> possible.  */
>>>> -            initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
>>>> +            if (usub64_overflow(ram_size, initrd_size, 
>>>> &initrd_base)) {
>>>> +                error_report("initial ram disk exceeds allotted ram 
>>>> size");
>>>> +                exit(1);
>>>> +            }
>>>> +            initrd_base &= TARGET_PAGE_MASK;
>>>> +            if (initrd_base <= kernel_high) {
>>>> +                warn_report("initial ram disk overlaps with kernel");
>>>> +            }
>>>
>>> Why is the first an error and the second a warning?
>>>
>>
>> Initially, I had both as fatal, but then I figured a (mis)adventurous
>> user may actually want to experiment with kernel/initrd overlap, whereas
>> an underflow is something we can't allow (and initrd being larger than
>> RAM is physically impossible AFAIK). I could make the overlap fatal,
>> too, if that's better?
> 
> I can't see it being useful at all.
> 

Heh, fair enough; fatal, it is, then.

Thanks,
Yodel

> 
> r~
>