Messages in this thread | | | Date | Wed, 24 Apr 2013 12:53:49 +0530 | From | Vasant Hegde <> | Subject | Re: [PATCH] PowerPC: kernel: memory access violation when rtas_data_buf contents are more than 1026 |
| |
On 04/24/2013 12:33 PM, Chen Gang wrote: > On 2013年04月24日 14:28, Vasant Hegde wrote: >> On 04/23/2013 08:42 AM, Chen Gang wrote: >>> >>> need set '\0' for 'local_buffer'. >>> >>> SPLPAR_MAXLENGTH is 1026, RTAS_DATA_BUF_SIZE is 4096. so the contents of >>> rtas_data_buf may truncated in memcpy. >>> >>> if contents are really truncated. >>> the splpar_strlen is more than 1026. the next while loop checking will >>> not find the end of buffer. that will cause memory access violation. >>> >> >> Per parameter length in ibm,get-system-parameter RTAS call is limited to >> 1026 bytes (1024 bytes of data + 2 bytes length). And 'rtas_data_buf' >> was set to 0 (first 1026 bytes) before call RTAS call. At the worst if >> we get junk in RTAS output length field helps to exit from the while >> loop. So I don't think we need this patch. > > Is get-system-parameter return the NUL terminated string ? if so, it > will no issue (just like your discription). >
Length includes the length of the NULL. So (idx < splpar_strlen) is safe. IMO existing code is proper.
-Vasant
> If it will not return NUL terminated string, please see line 326: > > "while ((*local_buffer)&& (idx< splpar_strlen))" > (when idx == 1024, *local_buffer is memory access violation). > > Since we use the first 2 bytes as length, and also be sure of the real > length will never more than 1024, I suggest to: > > ---------------------------patch begin-------------------------------- > > diff --git a/arch/powerpc/kernel/lparcfg.c b/arch/powerpc/kernel/lparcfg.c > index 801a757..f8bd7cf 100644 > --- a/arch/powerpc/kernel/lparcfg.c > +++ b/arch/powerpc/kernel/lparcfg.c > @@ -323,7 +323,7 @@ static void parse_system_parameter_string(struct seq_file *m) > > w_idx = 0; > idx = 0; > - while ((*local_buffer)&& (idx< splpar_strlen)) { > + while (idx< splpar_strlen) { > workbuffer[w_idx++] = local_buffer[idx++]; > if ((local_buffer[idx] == ',') > || (local_buffer[idx] == '\0')) { > > ---------------------------patch end---------------------------------- > > Thanks. >
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |