Control: tag -1 + patch On Tue, Oct 12, 2021 at 05:40:42PM +1000, Alexander Zangerl wrote: > sorry, but i disagree. yes, it is a bug. but no, it is not severity > important: a 2000+ levels deep directory hierarchy has no > practical relevance whatsoever. > and a workaround exists: don't create ridiculously deep hierarchies ;-) I mean, yeah, fair enough; I have this explicitly for testing coreutils.
Below you'll find a patch for 0.4b47-2, replacing the static MAXPATHLEN path buffer in treescan() with a dynamically allocated one. This fixes both the original bug (down to directory depths I assume no-one has achieved before), and the ./[very long path redacted]: name exceeds 4096 char diagnostic I got when I originally bumped ulimit -s. It does use asprintf(3) ‒ while an extension, it's universal across the extant Berkeley and Solaris derivatives. -x mode still panics since the paths *are* longer than PATH_MAX, but you can't pass those to syscalls anyway and restructuring around that would be non-trivial (in this case, your workaround should be applied instead :P). Best, наб
Date: Wed, 29 Dec 2021 16:26:07 +0100
From: наб <nabijaczlew...@nabijaczleweli.xyz>
Subject: Don't trust MAXPATHLEN in treescan()
Dynamically allocate the path buffer:
this both allows paths longer than PATH_MAX,
and thins the stack frame considerably
(no longer segfaulting on a Very Deep tree
on Debian-default 8k ulimit -s)
--- dump-0.4b47.orig/restore/dirs.c
+++ dump-0.4b47/restore/dirs.c
@@ -287,9 +287,8 @@ treescan(char *pname, dump_ino_t ino, lo
{
struct inotab *itp;
struct direct *dp;
- int namelen;
off_t bpt;
- char locname[MAXPATHLEN + 1];
+ char *locname;
itp = inotablookup(ino);
if (itp == NULL) {
@@ -308,9 +307,6 @@ treescan(char *pname, dump_ino_t ino, lo
* begin search through the directory
* skipping over "." and ".."
*/
- namelen = snprintf(locname, sizeof(locname), "%s/", pname);
- if (namelen >= (int)sizeof(locname))
- namelen = sizeof(locname) - 1;
rst_seekdir(dirp, itp->t_seekpt, itp->t_seekpt);
dp = rst_readdir(dirp); /* "." */
if (dp != NULL && strcmp(dp->d_name, ".") == 0)
@@ -328,15 +324,13 @@ treescan(char *pname, dump_ino_t ino, lo
* a zero inode signals end of directory
*/
while (dp != NULL) {
- locname[namelen] = '\0';
- if (namelen + dp->d_namlen >= (int)sizeof(locname)) {
- fprintf(stderr, "%s%s: name exceeds %ld char\n",
- locname, dp->d_name, (long)sizeof(locname) - 1);
- } else {
- (void) strncat(locname, dp->d_name, (int)dp->d_namlen);
- treescan(locname, dp->d_ino, todo);
- rst_seekdir(dirp, bpt, itp->t_seekpt);
+ if (asprintf(&locname, "%s/%.*s", pname, (int)dp->d_namlen,
dp->d_name) == -1) {
+ panic("Error: %s\n", strerror(errno));
+ abort();
}
+ treescan(locname, dp->d_ino, todo);
+ free(locname);
+ rst_seekdir(dirp, bpt, itp->t_seekpt);
dp = rst_readdir(dirp);
bpt = rst_telldir(dirp);
}
signature.asc
Description: PGP signature
