On Tue, Jan 19, 2021 at 12:18:17PM -0800, Randy Dunlap wrote:
> On 1/19/21 11:45 AM, Kees Cook wrote:
> >
> > How about this so the base address is hashed once, with the offset added
> > to it for each line instead of each line having a "new" hash that makes
> > no sense:
>
> Yes, good patch. Should have been like this to begin with IMO.
>
> > diff --git a/lib/hexdump.c b/lib/hexdump.c
> > index 9301578f98e8..20264828752d 100644
> > --- a/lib/hexdump.c
> > +++ b/lib/hexdump.c
> > @@ -242,12 +242,17 @@ void print_hex_dump(const char *level, const char
> > *prefix_str, int prefix_type,
> > const void *buf, size_t len, bool ascii)
> > {
> > const u8 *ptr = buf;
> > + const u8 *addr;
> > int i, linelen, remaining = len;
> > unsigned char linebuf[32 * 3 + 2 + 32 + 1];
> >
> > if (rowsize != 16 && rowsize != 32)
> > rowsize = 16;
> >
> > + if (prefix_type == DUMP_PREFIX_ADDRESS &&
> > + ptr_to_hashval(ptr, &addr))
> > + addr = 0;
> > +
> > for (i = 0; i < len; i += rowsize) {
> > linelen = min(remaining, rowsize);
> > remaining -= rowsize;
> > @@ -258,7 +263,7 @@ void print_hex_dump(const char *level, const char
> > *prefix_str, int prefix_type,
> > switch (prefix_type) {
> > case DUMP_PREFIX_ADDRESS:
> > printk("%s%s%p: %s\n",
> > - level, prefix_str, ptr + i, linebuf);
> > + level, prefix_str, addr + i, linebuf);
>
> Is 'addr' always set here?
> It is only conditionally set above.
It should be, yes. Though I agree, it's not obvious. ptr_to_hashval()
will write to it when returning 0. So if that fails, addr will have 0
written. Both only happen under DUMP_PREFIX_ADDRESS.
--
Kees Cook
