Re: [PATCH] cifs: fix rename() by ensuring source handle opened with DELETE bit

[Paulo Alcantara <pc@cjr.nz>]

Aurelien Aptel <aaptel@suse.com> writes:

> To rename a file in SMB2 we open it with the DELETE access and do a
> special SetInfo on it. If the handle is missing the DELETE bit the
> server will fail the SetInfo with STATUS_ACCESS_DENIED.
>
> We currently try to reuse any existing opened handle we have with
> cifs_get_writable_path(). That function looks for handles with WRITE
> access but doesn't check for DELETE, making rename() fail if it finds
> a handle to reuse. Simple reproducer below.
>
> To select handles with the DELETE bit, this patch adds a flag argument
> to cifs_get_writable_path() and find_writable_file() and the existing
> 'bool fsuid_only' argument is converted to a flag.
>
> The cifsFileInfo struct only stores the UNIX open mode but not the
> original SMB access flags. Since the DELETE bit is not mapped in that
> mode, this patch stores the access mask in cifs_fid on file open,
> which is accessible from cifsFileInfo.
>
> Simple reproducer:
>
> 	#include <stdio.h>
> 	#include <stdlib.h>
> 	#include <sys/types.h>
> 	#include <sys/stat.h>
> 	#include <fcntl.h>
> 	#include <unistd.h>
> 	#define E(s) perror(s), exit(1)
>
> 	int main(int argc, char *argv[])
> 	{
> 		int fd, ret;
> 		if (argc != 3) {
> 			fprintf(stderr, "Usage: %s A B\n"
> 			"create&open A in write mode, "
> 			"rename A to B, close A\n", argv[0]);
> 			return 0;
> 		}
>
> 		fd = openat(AT_FDCWD, argv[1], O_WRONLY|O_CREAT|O_SYNC, 0666);
> 		if (fd == -1) E("openat()");
>
> 		ret = rename(argv[1], argv[2]);
> 		if (ret) E("rename()");
>
> 		ret = close(fd);
> 		if (ret) E("close()");
>
> 		return ret;
> 	}
>
> $ gcc -o bugrename bugrename.c
> $ ./bugrename /mnt/a /mnt/b
> rename(): Permission denied
>
> Signed-off-by: Aurelien Aptel <aaptel@suse.com>
> ---
>  fs/cifs/cifsglob.h  |  7 +++++++
>  fs/cifs/cifsproto.h |  5 +++--
>  fs/cifs/cifssmb.c   |  3 ++-
>  fs/cifs/file.c      | 19 ++++++++++++-------
>  fs/cifs/inode.c     |  6 +++---
>  fs/cifs/smb1ops.c   |  2 +-
>  fs/cifs/smb2inode.c |  4 ++--
>  fs/cifs/smb2ops.c   |  3 ++-
>  fs/cifs/smb2pdu.c   |  1 +
>  9 files changed, 33 insertions(+), 17 deletions(-)

Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>