Programming

14220 readers

2 users here now

All things programming and coding related. Subcommunity of Technology.

This community's icon was made by Aaron Schneider, under the CC-BY-NC-SA 4.0 license.

founded 2 years ago

MODERATORS

alyaza@beehaw.org

Hexorg@beehaw.org

remington@beehaw.org

A fun exercise in re the design quality of the Linux kernel: How many lines of code do you think was the diff to add "-o nosymfollow" (ignore symlinks) as a mount option? (git.kernel.org)

submitted 2 years ago* (last edited 2 years ago) by mozz@mbin.grits.dev to c/programming@beehaw.org

4 comments fedilink hide all child comments

Think of a similar scope of change to a large codebase you're familiar with, for frame of reference.

you are viewing a single comment's thread
view the rest of the comments

[–] TheOctonaut@mander.xyz 1 points 2 years ago (1 children)

Yeah I get that but you dig deeper and that implantation was just throwing an error that needs to be handled elsewhere. The "real" code is what is handling that error.

But then we're back to act acknowledging a meaningful point of having commits that do one thing and do it well and understandably, and I'm back to appreciating the difference between the kernel and our app.

[–] mozz@mbin.grits.dev 1 points 2 years ago

There is no code somewhere else that needed to be written somewhere else to handle the error. It just started working because those lines were added. That's what I keep saying. That is the nature of the good design I am pleased by to give the example.

I'm such a stubborn person that I literally had a look in namei.c to see what happens from that return, to make sure. I was fairly confident that that's how it works, just because it's rare in kernel development to add a feature without the means to trigger it and then add the means to trigger it in a separate patch (why would it not just all go in at once?), but I had a look and here's what I saw:

get_link() returns ELOOP
In one case, vfs_readlink() returns an error straight to the user after going through the VFS layer
In the more complex case where we were actually trying to follow it to do some other operation, pick_link() sees the error and returns an error
Which is what gets returned from step_into()
step_into() gets called from a bunch of places, but I looked at them briefly and quickly searched for ELOOP and NOSYMFOLLOW and convinced myself pretty well that there isn't anywhere that handles the error any differently than it would any other we-can't-do-this error, and ultimately it's going to just return the failure to the user. Because why would it do anything different? It looks to me although I'm not sure like step_into() up to walk_component() up to link_path_walk() up to path_lookupat() or path_openat() is sort of the main path you'd get to from userspace via do_file_open() or similar functions, and they all plan to just return the refusal to follow the symlink back to userspace.

Where are you getting your assertion that obviously it's already implemented somewhere else separate from this patch?