[LTP] [PATCH v2] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c

[Wei Gao]

On Fri, Nov 14, 2025 at 09:58:56AM +0100, Petr Vorel wrote:
> Hi all,
> 
> ...
> > > @@ -192,63 +253,36 @@ static int purge_dir(const char *path, char **errptr)
> > >  	return ret_val;
> > >  }
> 
> > Again, there shouldn't be two function for the same job. There should be
> > only purge_dirat() and the tst_purge_dir() should call purge_dirat()
> > with AT_FDCWD as the dirfd.
> 
> +1. That is what I meant by "it'd be nice if we could use only rmobjat()" in v1.
> 
> > >  	} else {
> > > -		if (unlink(obj) < 0) {
> > > +		if (unlinkat(dir_fd, obj, 0) < 0) {
> > >  			if (errmsg != NULL) {
> > > -				sprintf(err_msg,
> > > -					"unlink(%s) failed; errno=%d: %s", obj,
> > > +				snprintf(err_msg, sizeof(err_msg),
> > > +					"unlinkat(%s) failed; errno=%d: %s", obj,
> > >  					errno, tst_strerrno(errno));
> > >  				*errmsg = err_msg;
> > >  			}
> > > @@ -305,7 +339,7 @@ void tst_tmpdir(void)
> > >  		tst_resm(TERRNO, "%s: chdir(%s) failed", __func__, TESTDIR);
> 
> > >  		/* Try to remove the directory */
> > > -		if (rmobj(TESTDIR, &errmsg) == -1) {
> > > +		if (rmobjat(0, TESTDIR, &errmsg) == -1) {
> > >  			tst_resm(TWARN, "%s: rmobj(%s) failed: %s",
> > >  				 __func__, TESTDIR, errmsg);
> > >  		}
> > > @@ -343,7 +377,7 @@ void tst_rmdir(void)
> > >  	/*
> > >  	 * Attempt to remove the "TESTDIR" directory, using rmobj().
> > >  	 */
> > > -	if (rmobj(TESTDIR, &errmsg) == -1) {
> > > +	if (rmobjat(0, TESTDIR, &errmsg) == -1) {
> > >  		tst_resm(TWARN, "%s: rmobj(%s) failed: %s",
> > >  			 __func__, TESTDIR, errmsg);
> > >  	}
> 
> > We should pass AT_FDCWD to the two rmobjat() here since it's possible to
> > pass relative path in the TMPDIR environment variable. Otherwise the
> > code will not work with e.g. TMPDIR="." ./test_foo
> 
> Very good catch. BTW we expect TMPDIR to be correct - without double quotes and
> trailing '/' which are example for shell tests which lead to failures in LTP
> NFS tests. That was fixed in:
> 
> 273c497935 ("tst_test.sh: Remove possible double/trailing slashes from TMPDIR")
> 
> Wouldn't be better just to normalize relative TMPDIR to absolute path? Simple
> realpath() would do the job, right?
> 
> e.g. this patch "swapon03: Remove grep dependency" [1] + fix attempt to swapoff
> leftover from previous run (when one does ctrl+C in previous run) expect that
> TMPDIR is absolute. I'll note it below the patch that either we change
> lib/tst_tmpdir.c to convert relative to absolute, or swapon03.c test needs to do
> it itself. I would prefer lib/tst_tmpdir.c do the job including normalizing the
> path (more tests will benefit/need it).
Also i guess we need use another patch for this?
> 
> Kind regards,
> Petr
> 
> [1] https://lore.kernel.org/ltp/20251106163500.1063704-6-pvorel@suse.cz/
>