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.
------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
_______________________________________________
Kgdb-bugreport mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
