Re: [PATCH] fsck: use xasprintf to avoid buffer overruns with an insane fs type

[Theodore Ts'o <tytso@mit.edu>]

On Fri, Feb 16, 2018 at 11:55:05AM -0400, Peter Cordes wrote:
> > -	static char prog[256];
> > +	static char *prog = NULL;
> 
>  You're allocating / freeing every time it's used, so it shouldn't be
> static anymore.

Good point.  Karels has already accepted the patch, but if he hasn't
pushed it out, maybe he can ammend it.

> It might be easier to just use snprintf to truncate long strings,
> instead of introducing dynamic allocation which requires explicit
> freeing.  OTOH xasprintf makes it re-entrant / thread-safe, at the
> cost of forcing the caller to care about memory management.  (And at
> the cost of efficiency: prog is allocated / freed inside the loop.)

That's certainly an alternative, and in fact that's how I fixed it in
the old version of fsck still shipping in e2fsprogs (there are a few
places that still use it).

My main reason for using it there is that in e2fsprogs we don't assume
the use of xasprintf and family (it's a GNU extension, and e2fsprogs
tries to support legacy platforms such as MacOS and Windows :-).

However, given that fsck does use xasprintf already (and so there is
no portability advantag) and it's a lot easier to prove (or at least
satisfy to oneself) that an attacker who is trying to play tricks
can't do something unexpected when using xasprintf versus snprintf, I
went with asprintf in my patch to util-linux.

Cheers,

					- Ted