Hi Jason, Yes, you are right. I forgot to move forward the bytes after the XORed char. Your new patch is correct.
Thanks Sonic >-----Original Message----- >From: Jason Wessel [mailto:[email protected]] >Sent: Tuesday, January 19, 2010 11:45 PM >To: Mike Frysinger; Zhang, Sonic >Cc: [email protected]; >[email protected] >Subject: Re: [PATCH] kgdb: have ebin2mem call probe_kernel_write once > >I don't think this patch works quite right. See below. > >Mike Frysinger wrote: >> From: Sonic Zhang <[email protected]> >> >> Rather than call probe_kernel_write() one byte at a time, >process the >> whole buffer locally and pass the entire result in one go. >This way, >> arches that need to do special handling based on the length >can do so, >> or we only end up calling memcpy() once. >> >> Signed-off-by: Sonic Zhang <[email protected]> >> Signed-off-by: Mike Frysinger <[email protected]> >> --- >> kernel/kgdb.c | 19 +++++++------------ >> 1 files changed, 7 insertions(+), 12 deletions(-) >> >> diff --git a/kernel/kgdb.c b/kernel/kgdb.c index 2eb517e..180a1d0 >> 100644 >> --- a/kernel/kgdb.c >> +++ b/kernel/kgdb.c >> @@ -396,22 +396,17 @@ int kgdb_mem2hex(char *mem, char *buf, >int count) >> */ >> static int kgdb_ebin2mem(char *buf, char *mem, int count) { >> - int err = 0; >> - char c; >> + int size = 0; >> + char *c = buf; >> >> while (count-- > 0) { >> - c = *buf++; >> - if (c == 0x7d) >> - c = *buf++ ^ 0x20; >> - >> - err = probe_kernel_write(mem, &c, 1); >> - if (err) >> - break; >> - >> - mem++; >> + if (c[size] == 0x7d) >> + c[size] = *buf++ ^ 0x20; >> > >It is ok in this instance to destroy the original buffer, it >is a subtle change vs the original function. > >The bigger problem is that you write corrupt memory values. >Where is the assignment that keeps pushing bytes into "c" >after you hit an escape byte? > >From the gdb manual: > >http://sourceware.org/gdb/current/onlinedocs/gdb/Overview.html > >--- >The binary data representation uses |7d| (ascii `}') as an >escape character. Any escaped byte is transmitted as the >escape character followed by the original character XORed with >|0x20|. For example, the byte |0x7d| would be transmitted as >the two bytes |0x7d 0x5d|. The bytes >|0x23| (ascii `#'), |0x24| (ascii `$'), and |0x7d| (ascii `}') must >always be escaped. >--- > >If the bytes you are processing in the array looked like when >count = 4: >0x04 0x7d 0x5d 0x04 0x7d 0x03 > >The final values in the array after the processing need to look like: >0x04 0x7d 0x04 0x23 > > >As far as I can tell this change does not do that properly. >It is going to write instead: >0x04 0x5d 0x5d 0x04 > > >> + buf++; >> + size++; >> } >> >> - return err; >> + return probe_kernel_write(mem, c, size); >> } >> >> /* >> > > >Perhaps you can consider the following patch instead, which >should have the correct result. > >Jason. > > > _______________________________________________ Uclinux-dist-devel mailing list [email protected] https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel
