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 -- To unsubscribe from this list: send the line "unsubscribe util-linux" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
