Skip to content

Commit

Permalink
cifs: improve symlink handling for smb2+
Browse files Browse the repository at this point in the history 
When creating inode for symlink, the client used to send below
requests to fill it in:

    * create+query_info+close (STATUS_STOPPED_ON_SYMLINK)
    * create(+reparse_flag)+query_info+close (set file attrs)
    * create+ioctl(get_reparse)+close (query reparse tag)

and then for every access to the symlink dentry, the ->link() method
would send another:

    * create+ioctl(get_reparse)+close (parse symlink)

So, in order to improve:

    (i) Get rid of unnecessary roundtrips and then resolve symlinks as
	follows:

        * create+query_info+close (STATUS_STOPPED_ON_SYMLINK +
	                           parse symlink + get reparse tag)
        * create(+reparse_flag)+query_info+close (set file attrs)

    (ii) Set the resolved symlink target directly in inode->i_link and
         use simple_get_link() for ->link() to simply return it.

Signed-off-by: Paulo Alcantara (SUSE) <[email protected]>
Reviewed-by: Ronnie Sahlberg <[email protected]>
Signed-off-by: Steve French <[email protected]>
  • Loading branch information
@pcacjr
pcacjr authored and Steve French committed Oct 13, 2022
1 parent 977bb65 commit 76894f3
Show file tree
Hide file tree
Showing 14 changed files with 451 additions and 453 deletions.
9 changes: 7 additions & 2 deletions fs/cifs/cifsfs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ cifs_alloc_inode(struct super_block *sb)
cifs_inode->epoch = 0;
spin_lock_init(&cifs_inode->open_file_lock);
generate_random_uuid(cifs_inode->lease_key);
cifs_inode->symlink_target = NULL;

/*
* Can not set i_flags here - they get immediately overwritten to zero
Expand All @@ -412,7 +413,11 @@ cifs_alloc_inode(struct super_block *sb)
static void
cifs_free_inode(struct inode *inode)
{
kmem_cache_free(cifs_inode_cachep, CIFS_I(inode));
struct cifsInodeInfo *cinode = CIFS_I(inode);

if (S_ISLNK(inode->i_mode))
kfree(cinode->symlink_target);
kmem_cache_free(cifs_inode_cachep, cinode);
}

static void
Expand Down Expand Up @@ -1139,7 +1144,7 @@ const struct inode_operations cifs_file_inode_ops = {
};

const struct inode_operations cifs_symlink_inode_ops = {
.get_link = cifs_get_link,
.get_link = simple_get_link,
.permission = cifs_permission,
.listxattr = cifs_listxattr,
};
Expand Down
46 changes: 36 additions & 10 deletions fs/cifs/cifsglob.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,19 @@ struct cifs_cred {
struct cifs_ace *aces;
};

struct cifs_open_info_data {
char *symlink_target;
union {
struct smb2_file_all_info fi;
struct smb311_posix_qinfo posix_fi;
};
};

static inline void cifs_free_open_info(struct cifs_open_info_data *data)
{
kfree(data->symlink_target);
}

/*
*****************************************************************
* Except the CIFS PDUs themselves all the
Expand Down Expand Up @@ -307,20 +320,20 @@ struct smb_version_operations {
int (*is_path_accessible)(const unsigned int, struct cifs_tcon *,
struct cifs_sb_info *, const char *);
/* query path data from the server */
int (*query_path_info)(const unsigned int, struct cifs_tcon *,
struct cifs_sb_info *, const char *,
FILE_ALL_INFO *, bool *, bool *);
int (*query_path_info)(const unsigned int xid, struct cifs_tcon *tcon,
struct cifs_sb_info *cifs_sb, const char *full_path,
struct cifs_open_info_data *data, bool *adjust_tz, bool *reparse);
/* query file data from the server */
int (*query_file_info)(const unsigned int, struct cifs_tcon *,
struct cifs_fid *, FILE_ALL_INFO *);
int (*query_file_info)(const unsigned int xid, struct cifs_tcon *tcon,
struct cifsFileInfo *cfile, struct cifs_open_info_data *data);
/* query reparse tag from srv to determine which type of special file */
int (*query_reparse_tag)(const unsigned int xid, struct cifs_tcon *tcon,
struct cifs_sb_info *cifs_sb, const char *path,
__u32 *reparse_tag);
/* get server index number */
int (*get_srv_inum)(const unsigned int, struct cifs_tcon *,
struct cifs_sb_info *, const char *,
u64 *uniqueid, FILE_ALL_INFO *);
int (*get_srv_inum)(const unsigned int xid, struct cifs_tcon *tcon,
struct cifs_sb_info *cifs_sb, const char *full_path, u64 *uniqueid,
struct cifs_open_info_data *data);
/* set size by path */
int (*set_path_size)(const unsigned int, struct cifs_tcon *,
const char *, __u64, struct cifs_sb_info *, bool);
Expand Down Expand Up @@ -369,8 +382,8 @@ struct smb_version_operations {
struct cifs_sb_info *, const char *,
char **, bool);
/* open a file for non-posix mounts */
int (*open)(const unsigned int, struct cifs_open_parms *,
__u32 *, FILE_ALL_INFO *);
int (*open)(const unsigned int xid, struct cifs_open_parms *oparms, __u32 *oplock,
void *buf);
/* set fid protocol-specific info */
void (*set_fid)(struct cifsFileInfo *, struct cifs_fid *, __u32);
/* close a file */
Expand Down Expand Up @@ -1123,6 +1136,7 @@ struct cifs_fattr {
struct timespec64 cf_mtime;
struct timespec64 cf_ctime;
u32 cf_cifstag;
char *cf_symlink_target;
};

/*
Expand Down Expand Up @@ -1385,6 +1399,7 @@ struct cifsFileInfo {
struct work_struct put; /* work for the final part of _put */
struct delayed_work deferred;
bool deferred_close_scheduled; /* Flag to indicate close is scheduled */
char *symlink_target;
};

struct cifs_io_parms {
Expand Down Expand Up @@ -1543,6 +1558,7 @@ struct cifsInodeInfo {
struct list_head deferred_closes; /* list of deferred closes */
spinlock_t deferred_lock; /* protection on deferred list */
bool lease_granted; /* Flag to indicate whether lease or oplock is granted. */
char *symlink_target;
};

static inline struct cifsInodeInfo *
Expand Down Expand Up @@ -2111,4 +2127,14 @@ static inline size_t ntlmssp_workstation_name_size(const struct cifs_ses *ses)
return sizeof(ses->workstation_name);
}

static inline void move_cifs_info_to_smb2(struct smb2_file_all_info *dst, const FILE_ALL_INFO *src)
{
memcpy(dst, src, (size_t)((u8 *)&src->AccessFlags - (u8 *)src));
dst->AccessFlags = src->AccessFlags;
dst->CurrentByteOffset = src->CurrentByteOffset;
dst->Mode = src->Mode;
dst->AlignmentRequirement = src->AlignmentRequirement;
dst->FileNameLength = src->FileNameLength;
}

#endif /* _CIFS_GLOB_H */
13 changes: 6 additions & 7 deletions fs/cifs/cifsproto.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,9 @@ extern int cifs_unlock_range(struct cifsFileInfo *cfile,
extern int cifs_push_mandatory_locks(struct cifsFileInfo *cfile);

extern void cifs_down_write(struct rw_semaphore *sem);
extern struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid,
struct file *file,
struct tcon_link *tlink,
__u32 oplock);
struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
struct tcon_link *tlink, __u32 oplock,
const char *symlink_target);
extern int cifs_posix_open(const char *full_path, struct inode **inode,
struct super_block *sb, int mode,
unsigned int f_flags, __u32 *oplock, __u16 *netfid,
Expand All @@ -200,9 +199,9 @@ extern int cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr);
extern struct inode *cifs_iget(struct super_block *sb,
struct cifs_fattr *fattr);

extern int cifs_get_inode_info(struct inode **inode, const char *full_path,
FILE_ALL_INFO *data, struct super_block *sb,
int xid, const struct cifs_fid *fid);
int cifs_get_inode_info(struct inode **inode, const char *full_path,
struct cifs_open_info_data *data, struct super_block *sb, int xid,
const struct cifs_fid *fid);
extern int smb311_posix_get_inode_info(struct inode **pinode, const char *search_path,
struct super_block *sb, unsigned int xid);
extern int cifs_get_inode_info_unix(struct inode **pinode,
Expand Down
30 changes: 11 additions & 19 deletions fs/cifs/dir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,9 @@ check_name(struct dentry *direntry, struct cifs_tcon *tcon)

/* Inode operations in similar order to how they appear in Linux file fs.h */

static int
cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
struct tcon_link *tlink, unsigned oflags, umode_t mode,
__u32 *oplock, struct cifs_fid *fid)
static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
struct tcon_link *tlink, unsigned int oflags, umode_t mode, __u32 *oplock,
struct cifs_fid *fid, struct cifs_open_info_data *buf)
{
int rc = -ENOENT;
int create_options = CREATE_NOT_DIR;
Expand All @@ -177,7 +176,6 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
struct cifs_tcon *tcon = tlink_tcon(tlink);
const char *full_path;
void *page = alloc_dentry_path();
FILE_ALL_INFO *buf = NULL;
struct inode *newinode = NULL;
int disposition;
struct TCP_Server_Info *server = tcon->ses->server;
Expand Down Expand Up @@ -290,12 +288,6 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
goto out;
}

buf = kmalloc(sizeof(FILE_ALL_INFO), GFP_KERNEL);
if (buf == NULL) {
rc = -ENOMEM;
goto out;
}

/*
* if we're not using unix extensions, see if we need to set
* ATTR_READONLY on the create call
Expand Down Expand Up @@ -364,8 +356,7 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
{
#endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */
/* TODO: Add support for calling POSIX query info here, but passing in fid */
rc = cifs_get_inode_info(&newinode, full_path, buf, inode->i_sb,
xid, fid);
rc = cifs_get_inode_info(&newinode, full_path, buf, inode->i_sb, xid, fid);
if (newinode) {
if (server->ops->set_lease_key)
server->ops->set_lease_key(newinode, fid);
Expand Down Expand Up @@ -402,7 +393,6 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
d_add(direntry, newinode);

out:
kfree(buf);
free_dentry_path(page);
return rc;

Expand All @@ -427,6 +417,7 @@ cifs_atomic_open(struct inode *inode, struct dentry *direntry,
struct cifs_pending_open open;
__u32 oplock;
struct cifsFileInfo *file_info;
struct cifs_open_info_data buf = {};

if (unlikely(cifs_forced_shutdown(CIFS_SB(inode->i_sb))))
return -EIO;
Expand Down Expand Up @@ -484,8 +475,7 @@ cifs_atomic_open(struct inode *inode, struct dentry *direntry,
cifs_add_pending_open(&fid, tlink, &open);

rc = cifs_do_create(inode, direntry, xid, tlink, oflags, mode,
&oplock, &fid);

&oplock, &fid, &buf);
if (rc) {
cifs_del_pending_open(&open);
goto out;
Expand All @@ -510,7 +500,7 @@ cifs_atomic_open(struct inode *inode, struct dentry *direntry,
file->f_op = &cifs_file_direct_ops;
}

file_info = cifs_new_fileinfo(&fid, file, tlink, oplock);
file_info = cifs_new_fileinfo(&fid, file, tlink, oplock, buf.symlink_target);
if (file_info == NULL) {
if (server->ops->close)
server->ops->close(xid, tcon, &fid);
Expand All @@ -526,6 +516,7 @@ cifs_atomic_open(struct inode *inode, struct dentry *direntry,
cifs_put_tlink(tlink);
out_free_xid:
free_xid(xid);
cifs_free_open_info(&buf);
return rc;
}

Expand All @@ -547,6 +538,7 @@ int cifs_create(struct user_namespace *mnt_userns, struct inode *inode,
struct TCP_Server_Info *server;
struct cifs_fid fid;
__u32 oplock;
struct cifs_open_info_data buf = {};

cifs_dbg(FYI, "cifs_create parent inode = 0x%p name is: %pd and dentry = 0x%p\n",
inode, direntry, direntry);
Expand All @@ -565,11 +557,11 @@ int cifs_create(struct user_namespace *mnt_userns, struct inode *inode,
if (server->ops->new_lease_key)
server->ops->new_lease_key(&fid);

rc = cifs_do_create(inode, direntry, xid, tlink, oflags, mode,
&oplock, &fid);
rc = cifs_do_create(inode, direntry, xid, tlink, oflags, mode, &oplock, &fid, &buf);
if (!rc && server->ops->close)
server->ops->close(xid, tcon, &fid);

cifs_free_open_info(&buf);
cifs_put_tlink(tlink);
out_free_xid:
free_xid(xid);
Expand Down
41 changes: 22 additions & 19 deletions fs/cifs/file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,16 +209,14 @@ int cifs_posix_open(const char *full_path, struct inode **pinode,
}
#endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */

static int
cifs_nt_open(const char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
struct cifs_tcon *tcon, unsigned int f_flags, __u32 *oplock,
struct cifs_fid *fid, unsigned int xid)
static int cifs_nt_open(const char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
struct cifs_tcon *tcon, unsigned int f_flags, __u32 *oplock,
struct cifs_fid *fid, unsigned int xid, struct cifs_open_info_data *buf)
{
int rc;
int desired_access;
int disposition;
int create_options = CREATE_NOT_DIR;
FILE_ALL_INFO *buf;
struct TCP_Server_Info *server = tcon->ses->server;
struct cifs_open_parms oparms;

Expand Down Expand Up @@ -255,10 +253,6 @@ cifs_nt_open(const char *full_path, struct inode *inode, struct cifs_sb_info *ci

/* BB pass O_SYNC flag through on file attributes .. BB */

buf = kmalloc(sizeof(FILE_ALL_INFO), GFP_KERNEL);
if (!buf)
return -ENOMEM;

/* O_SYNC also has bit for O_DSYNC so following check picks up either */
if (f_flags & O_SYNC)
create_options |= CREATE_WRITE_THROUGH;
Expand All @@ -276,9 +270,8 @@ cifs_nt_open(const char *full_path, struct inode *inode, struct cifs_sb_info *ci
oparms.reconnect = false;

rc = server->ops->open(xid, &oparms, oplock, buf);

if (rc)
goto out;
return rc;

/* TODO: Add support for calling posix query info but with passing in fid */
if (tcon->unix_ext)
Expand All @@ -294,8 +287,6 @@ cifs_nt_open(const char *full_path, struct inode *inode, struct cifs_sb_info *ci
rc = -EOPENSTALE;
}

out:
kfree(buf);
return rc;
}

Expand Down Expand Up @@ -325,9 +316,9 @@ cifs_down_write(struct rw_semaphore *sem)

static void cifsFileInfo_put_work(struct work_struct *work);

struct cifsFileInfo *
cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
struct tcon_link *tlink, __u32 oplock)
struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
struct tcon_link *tlink, __u32 oplock,
const char *symlink_target)
{
struct dentry *dentry = file_dentry(file);
struct inode *inode = d_inode(dentry);
Expand All @@ -347,6 +338,15 @@ cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
return NULL;
}

if (symlink_target) {
cfile->symlink_target = kstrdup(symlink_target, GFP_KERNEL);
if (!cfile->symlink_target) {
kfree(fdlocks);
kfree(cfile);
return NULL;
}
}

INIT_LIST_HEAD(&fdlocks->locks);
fdlocks->cfile = cfile;
cfile->llist = fdlocks;
Expand Down Expand Up @@ -440,6 +440,7 @@ static void cifsFileInfo_put_final(struct cifsFileInfo *cifs_file)
cifs_put_tlink(cifs_file->tlink);
dput(cifs_file->dentry);
cifs_sb_deactive(sb);
kfree(cifs_file->symlink_target);
kfree(cifs_file);
}

Expand Down Expand Up @@ -572,6 +573,7 @@ int cifs_open(struct inode *inode, struct file *file)
bool posix_open_ok = false;
struct cifs_fid fid;
struct cifs_pending_open open;
struct cifs_open_info_data data = {};

xid = get_xid();

Expand Down Expand Up @@ -662,15 +664,15 @@ int cifs_open(struct inode *inode, struct file *file)
if (server->ops->get_lease_key)
server->ops->get_lease_key(inode, &fid);

rc = cifs_nt_open(full_path, inode, cifs_sb, tcon,
file->f_flags, &oplock, &fid, xid);
rc = cifs_nt_open(full_path, inode, cifs_sb, tcon, file->f_flags, &oplock, &fid,
xid, &data);
if (rc) {
cifs_del_pending_open(&open);
goto out;
}
}

cfile = cifs_new_fileinfo(&fid, file, tlink, oplock);
cfile = cifs_new_fileinfo(&fid, file, tlink, oplock, data.symlink_target);
if (cfile == NULL) {
if (server->ops->close)
server->ops->close(xid, tcon, &fid);
Expand Down Expand Up @@ -712,6 +714,7 @@ int cifs_open(struct inode *inode, struct file *file)
free_dentry_path(page);
free_xid(xid);
cifs_put_tlink(tlink);
cifs_free_open_info(&data);
return rc;
}

Expand Down
Loading

0 comments on commit 76894f3

Please sign in to comment.