On 14 Mar 2016 16:15, Rich Felker wrote:
> On Mon, Mar 14, 2016 at 02:27:56PM -0400, Mike Frysinger wrote:
> > On 14 Mar 2016 18:11, Bartosz Gołaszewski wrote:
> > > 2016-03-14 15:27 GMT+01:00 Mike Frysinger <vap...@gentoo.org>:
> > > > On 14 Mar 2016 11:07, Bartosz Golaszewski wrote:
> > > >> +#ifndef __BB_NAMESPACE_H
> > > >> +#define __BB_NAMESPACE_H
> > > >
> > > > use a naming style like other busybox headers
> > > >
> > > >> +/*
> > > >> + * Longest possible path to a procfs file used in namespace utils.
> > > >> Must be
> > > >> + * able to contain the '/proc/' string, the '/ns/user' string which
> > > >> is the
> > > >> + * longest namespace name and a 32-bit integer representing the
> > > >> process ID.
> > > >> + */
> > > >> +#define NS_PROC_PATH_MAX (sizeof("/proc//ns/user") + sizeof(pid_t) *
> > > >> 3)
> > > >
> > > > using the sizeof pid_t as a proxy for how many chars it'd take to render
> > > > a decimal number in ASCII is wonky. just hardcode it as "10" since that
> > > > is the largest unsigned 32bit number ("4294967296").
> > >
> > > Can you elaborate on how it's wonky? While your solution is perfectly
> > > fine I think that there's nothing wrong with the way I've done it
> > > neither.
> >
> > the code seems to assume that the byte size scales into the number of
> > chars required to represent the number in base 10 when it's really a
> > log scale. here's a table to show it's wonky:
>
> sizeof is already a log scale. 3*sizeof(pid_t) is
> 3*log_256(MAX_PID+1), which is greater than 3*ceil(log_1000(MAX_PID)),
> the actual space requirement.
i was referring to log base 10 which is normally what log() represents.
the concern isn't with the limits that Linux happens to impose by default
(0x8000) but with making sure bad values don't screw everything up. just
because the linux kernel doesn't normally create a pid larger than 32k at
the moment doesn't mean bad values can't leak in somewhere else. these
pid #'s are being sourced directly from the user.
> > pid_t |sizeof |*3 val|actual|
> > size |(bytes)|(char)| |
> > (bits)| | | |
> > ------|-------|------|------|
> > 8 | 1 | 3 | 4 |
> > 16 | 2 | 6 | 6 |
> > 32 | 4 | 12 | 11 |
> > 64 | 8 | 24 | 20 |
> > 128 | 16 | 48 | 40 |
> >
> > the "actual" value is one higher than you might expect because pid_t
> > is a signed quantity. for 8bit, "-128" is 0x80 and takes 4 bytes.
> > for 32bit ("int"), "-2147483648" is 0x80000000 and takes 11 bytes.
>
> While pid_t is signed, negative values are not meaningful as pids;
> they're pgids or special sentinels in some contexts. If this code is
> trying to print a negative pid into a pathname used in /proc, that's a
> bug. But in general, when using the 3*sizeof idiom you should add +1
> for a sign if needed and another +1 for null termination if not
> already included elsewhere.
relying on full error checking all the time is a bad way to design
string buffers. "this func will smash the stack if you happen to
pass in a negative value" is terrible.
the NUL byte is technicallly already counted when you do the
sizeof("/proc/...").
but this esoteric logic is why i'm pushing for something better than
sprinkling ad-hoc logic everywhere and hoping for the best.
-mike
signature.asc
Description: Digital signature
_______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
