Tom Talpey <tom@xxxxxxxxxx> writes:
> On 4/16/2023 2:38 PM, Paulo Alcantara wrote:
>> @@ -34,19 +34,33 @@ static inline int dfs_get_referral(struct cifs_mount_ctx *mnt_ctx, const char *p
>> cifs_remap(cifs_sb), path, ref, tl);
>> }
>>
>> +/* Return DFS full path out of a dentry set for automount */
>> static inline char *dfs_get_automount_devname(struct dentry *dentry, void *page)
>> {
>> struct cifs_sb_info *cifs_sb = CIFS_SB(dentry->d_sb);
>> struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
>> struct TCP_Server_Info *server = tcon->ses->server;
>> + size_t len;
>> + char *s;
>>
>> if (unlikely(!server->origin_fullpath))
>> return ERR_PTR(-EREMOTE);
>>
>> - return __build_path_from_dentry_optional_prefix(dentry, page,
>> - server->origin_fullpath,
>> - strlen(server->origin_fullpath),
>> - true);
>> + s = dentry_path_raw(dentry, page, PATH_MAX);
>> + if (IS_ERR(s))
>> + return s;
>> + /* for root, we want "" */
>> + if (!s[1])
>> + s++;
>
> The above pointer increment is really hard to understand, given the
> comment. So, if the result is a single-character path, presumably "/",
> advance the pointer so it becomes a null string? It's not obvious from
> this code and comment.
I'll improve the comment to mention "/".
>> + len = strlen(server->origin_fullpath);
>> + if (s < (char *)page + len)
>> + return ERR_PTR(-ENAMETOOLONG);
>> +
>> + s -= len;
>
> This looks doubly dangerous. What prevents the pointer from moving
> backwards to ahead of the buffer? Especially in light of the above
> root-only adjustment?
dentry_path_raw() places the path name at the _end_ of provided @page
buffer and returns a pointer to it. The above if check should prevent
such case of happening.
