lib/vfscore: Fix double-lock bug in symlink

[andreittr]

Description of changes

This change fixes an issue where the lock of the destination parent directory was being taken more than once in the symlink syscall by using a locked variant of namei_last_nofollow, which this PR also adds. In implementing the locked version of the latter function there's also some factoring out of common code.

Prerequisite checklist

• Read the contribution guidelines regarding submitting new changes to the project;
• Tested your changes against relevant architectures and platforms;
• Ran the checkpatch.uk on your commit series before opening this PR;
• Updated relevant documentation.

checkpatch.uk complains, but I've purposely kept the style consistent within the file.

Base target

• Architecture(s): all
• Platform(s): all
• Application(s): all

Additional configuration

Test by issuing a symlink syscall; staging fails with a lock assert, fixed code behaves as expected.

[unikraft-bot]

❗ Checkpatch failed

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request but it encountered errors:

SHA	commit	checkpatch
193d645	lib/vfscore: Add locked namei_last_nofollow	❌
cd7f831	lib/vfscore: Fix double-lock bug in symlink	✅

Truncated logs starting from first error 193d645:

WARNING:BRACES: braces {} are not necessary for single statement blocks
#27: FILE: lib/vfscore/lookup.c:246:
+	if (path[0] != '/') {
+		return (ENOTDIR);
+	}

ERROR:RETURN_PARENTHESES: return is not a function, parentheses are not required
#28: FILE: lib/vfscore/lookup.c:247:
+		return (ENOTDIR);

WARNING:USE_NEGATIVE_ERRNO: return of an errno should typically be negative (ie: return -ENOTDIR)
#28: FILE: lib/vfscore/lookup.c:247:
+		return (ENOTDIR);

WARNING:LINE_SPACING: Missing a blank line after declarations
#32: FILE: lib/vfscore/lookup.c:251:
+	char *name = strrchr(path, '/');
+	if (name == NULL) {

WARNING:BRACES: braces {} are not necessary for single statement blocks
#32: FILE: lib/vfscore/lookup.c:251:
+	if (name == NULL) {
+		return (ENOENT);
+	}

ERROR:RETURN_PARENTHESES: return is not a function, parentheses are not required
#33: FILE: lib/vfscore/lookup.c:252:
+		return (ENOENT);

WARNING:USE_NEGATIVE_ERRNO: return of an errno should typically be negative (ie: return -ENOENT)
#33: FILE: lib/vfscore/lookup.c:252:
+		return (ENOENT);

WARNING:LINE_SPACING: Missing a blank line after declarations
#39: FILE: lib/vfscore/lookup.c:258:
+	int error = vfs_findroot(path, mp, &p);
+	if (error != 0) {

WARNING:BRACES: braces {} are not necessary for single statement blocks
#39: FILE: lib/vfscore/lookup.c:258:
+	if (error != 0) {
+		return (ENOTDIR);
+	}

ERROR:RETURN_PARENTHESES: return is not a function, parentheses are not required
#40: FILE: lib/vfscore/lookup.c:259:
+		return (ENOTDIR);

WARNING:USE_NEGATIVE_ERRNO: return of an errno should typically be negative (ie: return -ENOTDIR)
#40: FILE: lib/vfscore/lookup.c:259:
+		return (ENOTDIR);

WARNING:LONG_LINE_COMMENT: line over 80 characters
#45: FILE: lib/vfscore/lookup.c:264:
+	// We want to treat things like /tmp/ the same as /tmp. Best way to do that

WARNING:LINE_SPACING: Missing a blank line after declarations
#48: FILE: lib/vfscore/lookup.c:267:
+	int l = strlen(node) - 1;
+	if (l && node[l] == '/') {

WARNING:BRACES: braces {} are not necessary for single statement blocks
#48: FILE: lib/vfscore/lookup.c:267:
+	if (l && node[l] == '/') {
+		node[l] = '\0';
+	}

WARNING:BRACES: braces {} are not necessary for single statement blocks
#71: FILE: lib/vfscore/lookup.c:291:
+	if ((error = _namei_prep_path(path, &mp, node, &name))) {
+		return error;
 	}

WARNING:BRACES: braces {} are not necessary for single statement blocks
#79: FILE: lib/vfscore/lookup.c:299:
+		if (error != 0) {
+			goto out;
+		}

ERROR:RETURN_PARENTHESES: return is not a function, parentheses are not required
#106: FILE: lib/vfscore/lookup.c:316:
+	return (error);

WARNING:BRACES: braces {} are not necessary for single statement blocks
#133: FILE: lib/vfscore/lookup.c:337:
+	if ((error = _namei_prep_path(path, &mp, node, &name))) {
+		return error;
+	}

total: 4 errors, 14 warnings, 150 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/tmp/build/a53d4c5b/patches/0001-lib-vfscore-Add-locked-namei_last_nofollow.patch has style problems, please review.

NOTE: Ignored message types: ASSIGN_IN_IF FILE_PATH_CHANGES NEW_TYPEDEFS OBSOLETE

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

---

View complete logs | Learn more about Unikraft's coding style and contribution guidelines.

[mariasfiraiala]

Thanks @andreittr, this works as expected. Looks good too.

Reviewed-by: Maria Sfiraiala maria.sfiraiala@gmail.com

[razvand]

Approved-by: Razvan Deaconescu razvand@unikraft.io