Skip to content

Commit fe05a56

Browse files
author
Francesco Lavra
committed
filesystem: Impose global recursion limit when resolving symbolic links
Mutual recursion between filesystem_resolve_sstring() and filesystem_follow_links() consumes ~540 bytes on the kernel stack per level. SYMLINK_HOPS_MAX=8 limits each individual follow_links call but each nested intermediate path component gets its own hop counter with no global recursion limit. This causes a stack overflow when handling a stat() syscall with a sufficiently deep symlink chain. Fix this issue by modifying filesystem_follow_links() so that the hop count variable is a function parameter (whose value is incremented at each recursion level) instead of a local variable. Modify the SYMLINK_HOPS_MAX constant from 8 to 40 (which matches the corresponding constant in the Linux kernel), since this constant now represents the limit on the cumulative number of links (instead of the number of "consecutive" links) when resolving a filesystem path.
1 parent 6d1691a commit fe05a56

2 files changed

Lines changed: 18 additions & 17 deletions

File tree

src/fs/fs.c

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,11 +1164,9 @@ static tuple lookup_follow(filesystem *fs, tuple t, string a, tuple *p)
11641164
* is updated to point to the new filesystem.
11651165
* The refcount of the filesystem returned via the 'fs' pointer is incremented. */
11661166
// fused buffer wrap, split, and resolve
1167-
int filesystem_resolve_sstring(filesystem *fs, tuple cwd, sstring f, tuple *entry,
1168-
tuple *parent)
1167+
static int filesystem_resolve_internal(filesystem *fs, tuple cwd, sstring f,
1168+
tuple *entry, tuple *parent, int chain_depth)
11691169
{
1170-
assert(fs_path_helper.get_root_fs);
1171-
11721170
tuple t;
11731171
if (!sstring_is_empty(f) && (f.ptr[0] == '/')) {
11741172
filesystem root_fs = fs_path_helper.get_root_fs();
@@ -1206,7 +1204,7 @@ int filesystem_resolve_sstring(filesystem *fs, tuple cwd, sstring f, tuple *entr
12061204
err = -ENOENT;
12071205
goto done;
12081206
}
1209-
err = filesystem_follow_links(fs, t, p, &t);
1207+
err = filesystem_follow_links(fs, t, p, chain_depth, &t);
12101208
if (err) {
12111209
t = false;
12121210
goto done;
@@ -1244,6 +1242,13 @@ int filesystem_resolve_sstring(filesystem *fs, tuple cwd, sstring f, tuple *entr
12441242
return (t ? 0 : err);
12451243
}
12461244

1245+
int filesystem_resolve_sstring(filesystem *fs, tuple cwd, sstring f, tuple *entry,
1246+
tuple *parent)
1247+
{
1248+
assert(fs_path_helper.get_root_fs);
1249+
return filesystem_resolve_internal(fs, cwd, f, entry, parent, 0);
1250+
}
1251+
12471252
/* Same as filesystem_resolve_sstring(), but if the path resolves to a symbolic link, the link is
12481253
* followed. */
12491254
int filesystem_resolve_sstring_follow(filesystem *fs, tuple cwd, sstring f, tuple *entry,
@@ -1252,7 +1257,7 @@ int filesystem_resolve_sstring_follow(filesystem *fs, tuple cwd, sstring f, tupl
12521257
tuple t, p;
12531258
int ret = filesystem_resolve_sstring(fs, cwd, f, &t, &p);
12541259
if (!ret) {
1255-
ret = filesystem_follow_links(fs, t, p, &t);
1260+
ret = filesystem_follow_links(fs, t, p, 0, &t);
12561261
}
12571262
if ((ret == 0) && entry) {
12581263
*entry = t;
@@ -1263,35 +1268,31 @@ int filesystem_resolve_sstring_follow(filesystem *fs, tuple cwd, sstring f, tupl
12631268
return ret;
12641269
}
12651270

1266-
#define SYMLINK_HOPS_MAX 8
1271+
#define SYMLINK_HOPS_MAX 40
12671272

1268-
int filesystem_follow_links(filesystem *fs, tuple link, tuple parent,
1273+
int filesystem_follow_links(filesystem *fs, tuple link, tuple parent, int hop_count,
12691274
tuple *target)
12701275
{
12711276
if (!is_symlink(link)) {
12721277
return 0;
12731278
}
12741279

12751280
tuple target_t;
1276-
int hop_count = 0;
12771281
while (true) {
12781282
buffer target_b = linktarget(link);
12791283
if (!target_b) {
12801284
*target = link;
12811285
return 0;
12821286
}
1287+
if (hop_count++ == SYMLINK_HOPS_MAX)
1288+
return -ELOOP;
12831289
filesystem prev = *fs;
1284-
int ret = filesystem_resolve_sstring(fs, parent, buffer_to_sstring(target_b), &target_t,
1285-
&parent);
1290+
int ret = filesystem_resolve_internal(fs, parent, buffer_to_sstring(target_b),
1291+
&target_t, &parent, hop_count);
12861292
filesystem_release(prev);
12871293
if (ret) {
12881294
return ret;
12891295
}
1290-
if (is_symlink(target_t)) {
1291-
if (hop_count++ == SYMLINK_HOPS_MAX) {
1292-
return -ELOOP;
1293-
}
1294-
}
12951296
link = target_t;
12961297
}
12971298
}

src/fs/fs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ int filesystem_resolve_sstring(filesystem *fs, tuple cwd, sstring f, tuple *entr
274274
int filesystem_resolve_sstring_follow(filesystem *fs, tuple cwd, sstring f, tuple *entry,
275275
tuple *parent);
276276

277-
int filesystem_follow_links(filesystem *fs, tuple link, tuple parent,
277+
int filesystem_follow_links(filesystem *fs, tuple link, tuple parent, int hop_count,
278278
tuple *target);
279279

280280
int file_get_path(filesystem fs, inode ino, char *buf, u64 len);

0 commit comments

Comments
 (0)