To reproduce: 1. mount overlay fs 2. create a non-empty directory 3. chdir to the non-empty directory 4. try rmdir the non-empty current working directory (should fail due to non-empty) 5. call getcwd. The call will fail with "No such file or directory", but it should not, becuase the directory was not successfully removed. Example: # mkdir lower upper work mnt # mount -t overlay -o upperdir=upper,lowerdir=lower,workdir=work overlay mnt # mkdir -p mnt/a/b # cd mnt/a # rmdir ../a rmdir: failed to remove ‘../a’: Directory not empty ash: getcwd: No such file or directory
More details: https://github.com/docker/docker/issues/19082
Created attachment 199071 [details] propsed patch
A patch is proposed at: https://lkml.org/lkml/2016/1/8/482 I've also attached it here.
Did you test the patch? It does not seem to solve the issue for me. I am not familiar with the linux filesystem code, but I would have expected it to also be dropped in the -ESTALE case? Also, will this introduce a leak?
Yes I did test it. d_drop() isn't called in the -ESTALE case. It's if(!err), not if (err). d_drop() is called only if err is zero. What kind of leak do you think it may introduce? Thanks
(In reply to ncopa from comment #4) > Did you test the patch? It does not seem to solve the issue for me. > > I am not familiar with the linux filesystem code, but I would have expected > it to also be dropped in the -ESTALE case? > > Also, will this introduce a leak? I see. The original code do d_drop() in the -ESTALE case, but this should be unrelated. If this doesn't work then there may be another bug somewhere, e.g. your code path does not call ovl_remove_upper()? How did you reproduce the problem with this patch compiled? I tried it on overlayfs over xfs/ext4/ext3 by following your steps. All worked.
(In reply to Rui Wang from comment #6) > (In reply to ncopa from comment #4) > > Did you test the patch? It does not seem to solve the issue for me. > > > > I am not familiar with the linux filesystem code, but I would have expected > > it to also be dropped in the -ESTALE case? > > > > Also, will this introduce a leak? > > I see. The original code do d_drop() in the -ESTALE case, but this should be > unrelated. If this doesn't work then there may be another bug somewhere, > e.g. your code path does not call ovl_remove_upper()? How did you reproduce > the problem with this patch compiled? I tried it on overlayfs over > xfs/ext4/ext3 by following your steps. All worked. I must have done something wrong during my test and tested wrong version. It works. Tested on overlay over tmpfs. > What kind of leak do you think it may introduce? I don't know. I am not familiar with the filesystem code. It just looks that something that previously was dropped (possibly free'ed/released?) no longer is, so I just want be sure. But if you know that no leak was introduced, then I'm all happy :) I will push this for the Alpine Linux kernel. Thanks!
(In reply to ncopa from comment #7) > > What kind of leak do you think it may introduce? > > I don't know. I am not familiar with the filesystem code. It just looks that > something that previously was dropped (possibly free'ed/released?) no longer > is, so I just want be sure. > > But if you know that no leak was introduced, then I'm all happy :) I think there should be no problem. -ESTALE means it's already deleted. Then it has been droped already and shouldn't really be still on the hash queue. > I will push this for the Alpine Linux kernel. Cool. Thanks.