netdev - Re: [PATCH net-next v3 1/3] hexdump: Implement macro for converting large buffers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z7jLE-GKWPPn-cBT@li-4c4c4544-0047-5210-804b-b8c04f323634.ibm.com>
Date: Fri, 21 Feb 2025 12:50:59 -0600
From: Nick Child <nnac123@...ux.ibm.com>
To: David Laight <david.laight.linux@...il.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, horms@...nel.org,
        nick.child@....com, pmladek@...e.com, rostedt@...dmis.org,
        john.ogness@...utronix.de, senozhatsky@...omium.org
Subject: Re: [PATCH net-next v3 1/3] hexdump: Implement macro for converting
 large buffers

On Fri, Feb 21, 2025 at 06:04:35PM +0000, David Laight wrote:
> On Fri, 21 Feb 2025 11:37:46 -0600
> Nick Child <nnac123@...ux.ibm.com> wrote:
> > On Thu, Feb 20, 2025 at 10:00:50PM +0000, David Laight wrote:
> > > You could do:
> > > #define for_each_line_in_hex_dump(buf_offset, rowsize, linebuf, linebuflen, groupsize, buf, len, ascii) \
> > > for (unsigned int _offset = 0, _rowsize = (rowsize), _len = (len); \
> > > 	((offset) = _offset) < _len && (hex_dump_to_buffer((const char *)(buf) + _offset, _len - _offset, \
>           ^ needs to be buf_offset.
> 
> > > 		_rowsize, (groupsize), (linebuf), (linebuflen), (ascii)), 1); \
> > > 	_offset += _rowsize )
> > > 
> > > (Assuming I've not mistyped it.)
> > >   
> >
> > Trying to understand the reasoning for declaring new tmp variables;
> > Is this to prevent the values from changing in the body of the loop?
>
> No, it is to prevent side-effects happening more than once.
> Think about what would happen if someone passed 'foo -= 4' for len.
>

If we are protecting against those cases then linebuf, linebuflen,
groupsize and ascii should also be stored into tmp variables since they
are referenced in the loop conditional every iteration.
At which point the loop becomes too messy IMO.
Are any other for_each implementations taking these precautions?

Not trying to come off dismissive, I genuinely appreciate all the
insight, trying to learn more for next time.

> > I tried to avoid declaring new vars in this design because I thought it
> > would recive pushback due to possible name collision and variable
> > declaration inside for loop initializer.
>
> The c std level got upped recently to allow declarations inside loops.
> Usually for a 'loop iterator' - but I think you needed that to be
> exposed outsize the loop.
> (Otherwise you don't need _offset and buf_offset.
>

As in decrementing _len and increasing a _buf var rather than tracking
offset?
I don't really care for exposing the offset, during design I figured
some caller may make use of it but I think it is worth removing to reduce
the number of arguments.

Thanks again,
Nick