autofs 4.1.4 no-unlink patch
PATCH 1/1 autofs 4.1.4 no-unlink patch
Ian,
We've been hearing some reports of autofs removing entire home directories when mounts expire. It appears that this could be some sort of race condition in the walk_tree code, where when the function starts the directory is unmounted and then autofs unlinks files in the the directory that is supposed to be umounted. I've attached a patch to the rm_unwanted_fn which adds some additional checks before we unlink anything, and it prevents autofs from unlinking files (since I'm pretty sure that autofs never creates a regular file). Let me know what you think.
Thanks,
Chris
daemon/automount.c renamed from daemon/automount.c.no-unlink +28 -6
@@ -216,16 +216,38 @@ static int walk_tree(const char *base, istatic int rm_unwanted_fn (const char *file,const struct stat *st,int when,void *arg) {int rmsymlink = *(int *) arg; +struct stat newst;if (when ==0 ) {if (st->st_dev != ap.dev)return 0 ; - }else { -info ("rm_unwanted: %s\n" , file); -if (S_ISDIR (st->st_mode)) -rmdir (file); -else if (!S_ISLNK (st->st_mode) || rmsymlink) -unlink (file); +return 1 ; + } + +if (lstat (file, &newst)) { +crit ("rm_unwanted: unable to stat file, possible race " +"condition." ); +return 0 ; + } + +if (newst.st_dev != ap.dev) { +crit ("rm_unwanted: file %s has the wrong device, possible " +"race condition." ,file); +return 0 ; + } + +if (S_ISDIR (newst.st_mode)) { +if (rmdir (file)) { +info ("rm_unwanted: unable to remove directory" +" %s" , file); +return 0 ; + } + }else if (S_ISREG (newst.st_mode)) { +crit ("rm_unwanted: attempting to remove files from a mounted " +"directory." ); +return 0 ; + }else if (S_ISLNK (newst.st_mode) && rmsymlink) { +unlink (file); }return 1 ;
_______________________________________________
autofs mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/autofs
Ian Kent
maintainerQuoted 10 lines · Chris Feist
Ian,
We've been hearing some reports of autofs removing entire home directories when mounts expire. It appears that this could be some sort of race condition in the
walk_treecode, where when the function starts the directory is unmounted and then autofs unlinks files in the the directory that is supposed to be umounted. I've attached a patch to therm_unwanted_fnwhich adds some additional checks before we unlink anything, and it prevents autofs from unlinking files (since I'm pretty sure that autofs never creates a regular file). Let me know what you think.
Yes. There doesn't seem to be enough checking and the !S_ISLNK doesn't look quite right. Your right, the only objects in the autofs filesystem are directories and symlinks.
A mount happening during an expire might lead to this. But it shouldn't be able to happen due to the nature of the state machine.
I'll have to keep looking.
Ian
==> Regarding Re: [autofs] [PATCH] autofs 4.1.4 no-unlink patch; Ian Kent <[email protected]> adds:
Quoted 11 lines · Chris Feist
Quoted 11 lines
Ian,
We've been hearing some reports of autofs removing entire home directories when mounts expire. It appears that this could be some sort of race condition in the
walk_treecode, where when the function starts the directory is unmounted and then autofs unlinks files in the the directory that is supposed to be umounted. I've attached a patch to therm_unwanted_fnwhich adds some additional checks before we unlink anything, and it prevents autofs from unlinking files (since I'm pretty sure that autofs never creates a regular file). Let me know what you think.
raven> Yes. There doesn't seem to be enough checking and the !S_ISLNK raven> doesn't look quite right. Your right, the only objects in the autofs raven> filesystem are directories and symlinks.
raven> A mount happening during an expire might lead to this. But it raven> shouldn't be able to happen due to the nature of the state machine.
raven> I'll have to keep looking.
Right, it shouldn't be able to happen, I agree. However, we've seen 2 occurrences of this in the wild. Even if/when we fix this problem properly, I would lobby to put in these extra checks. They certainly don't hurt anything, and they make the code more robust. I certainly wouldn't like it if autofs deleted all of my files!
-Jeff
Ian Kent
maintainerQuoted 22 lines · Jeff Moyer
==> Regarding Re: [autofs] [PATCH] autofs 4.1.4 no-unlink patch; Ian Kent <[email protected]> adds:
Quoted 11 lines · Chris Feist
Quoted 11 lines
Ian,
We've been hearing some reports of autofs removing entire home directories when mounts expire. It appears that this could be some sort of race condition in the
walk_treecode, where when the function starts the directory is unmounted and then autofs unlinks files in the the directory that is supposed to be umounted. I've attached a patch to therm_unwanted_fnwhich adds some additional checks before we unlink anything, and it prevents autofs from unlinking files (since I'm pretty sure that autofs never creates a regular file). Let me know what you think.raven> Yes. There doesn't seem to be enough checking and the !S_ISLNK raven> doesn't look quite right. Your right, the only objects in the autofs raven> filesystem are directories and symlinks.
raven> A mount happening during an expire might lead to this. But it raven> shouldn't be able to happen due to the nature of the state machine.
Quoted 8 lines
raven> I'll have to keep looking.
Right, it shouldn't be able to happen, I agree. However, we've seen 2 occurrences of this in the wild. Even if/when we fix this problem properly, I would lobby to put in these extra checks. They certainly don't hurt anything, and they make the code more robust. I certainly wouldn't like it if autofs deleted all of my files!
Of course.
I'll have a look and publish it as an important patch.
It would be good to understand what's happening as well.
I've never seen this happen before. I wonder what changes we've done in this area lately. Mind that !S_ISLNK has been there since 4.0.0per?? and looks way wrong.
Ian