Re: [PATCH] Fix the EDD code misparsing the command line

From: H. Peter Anvin
Date: Mon Aug 28 2006 - 21:34:00 EST


Petr Vandrovec wrote:
+
+# Old-style boot protocol?
+old_cl:
+ push %ds # aka INITSEG
+ pop %fs
+
+ cmpw $0xa33f, (0x20)
+ jne done_cl # No command line at all?
+ movw (0x22), %si # Pointer relative to INITSEG

Perhaps you should convert ds:si to flat pointer and then this flat pointer to fs:si using method above, to avoid problems with dword access with si > 0xfffc or word access with si > 0xfffe ?

+
+# fs:si has the pointer to the command line now
+have_cl_pointer:
+ # loop through kernel command line one byte at a time
-cl_loop:
- cmpl $EDD_CL_EQUALS, (%si)
+cl_atspace:
+ movl %fs:(%si), %eax

This looks fine for new boot protocol, but what if old boot protocol puts command line so that its last byte is at INITSEG:0xffff ? You get #GP here, then, although command line is correctly zero terminated and does not overflow segment.


With the old protocol, the command line is supposed to fit inside the 64K segment, so I don't think that's an issue. Putting "Hail Mary" break at 0xfffd isn't a bad idea, though (especially since even if that is legitimate, we can't fit "edd=" into that one.)

If si is 0xfffb here, bad things happen. I know, things I've pointed out should not be problem in real world, and new code is definitely better than old one, but if you already have code to avoid endless loop if command line points to 64KB array of 0xFF let's do that right, no?

Agreed. I'll update the patch shortly.

-hpa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/