On 08/11/2016 03:38, Will Deacon wrote:
> On Tue, Oct 18, 2016 at 06:03:05PM +0200, G. Campana wrote:
>> ---
>> virtio/9p.c | 50 +++++++++++++++++++++++++-------------------------
>> 1 file changed, 25 insertions(+), 25 deletions(-)
>>
>> diff --git a/virtio/9p.c b/virtio/9p.c
>> index 5b2d261..3259b79 100644
>> --- a/virtio/9p.c
>> +++ b/virtio/9p.c
>> @@ -91,18 +91,6 @@ static struct p9_fid *get_fid(struct p9_dev *p9dev, int fid)
>> return new;
>> }
>>
>> -static int rel_to_abs(struct p9_dev *p9dev, const char *path, char *abs_path,
>> - size_t size)
>> -{
>> - int ret;
>> -
>> - ret = snprintf(abs_path, size, "%s/%s", p9dev->root_dir, path);
>> - if (ret >= (int)size)
>> - return -1;
>> -
>> - return 0;
>> -}
>
> Can this be merged with patch 5, where you introduced rel_to_abs?
>
Yes, I reworked this patch.
>> static void stat2qid(struct stat *st, struct p9_qid *qid)
>> {
>> *qid = (struct p9_qid) {
>> @@ -266,6 +254,28 @@ static int get_full_path(char *full_path, size_t size, struct p9_fid *fid,
>> return 0;
>> }
>>
>> +static int stat_rel(struct p9_dev *p9dev, const char *path, struct stat *st)
>> +{
>> + int ret;
>> + char full_path[PATH_MAX];
>> +
>> + ret = snprintf(full_path, sizeof(full_path), "%s/%s", p9dev->root_dir, path);
>> + if (ret >= (int)sizeof(full_path)) {
>> + errno = ENAMETOOLONG;
>> + return -1;
>> + }
>> +
>> + if (path_is_illegal(full_path)) {
>> + errno = EACCES;
>> + return -1;
>> + }
>
> Up to this point, you've just reimplemented most of get_full_path. Is it
> worth giving these two functions a comment "concatenate these two path
> components and check if the result is legal" backend?
>
I introduced get_full_path_helper(), which is called by stat_rel() and
get_full_path().
>> + if (lstat(full_path, st) != 0)
>> + return -1;
>> +
>> + return 0;
>> +}
>> +
>> static void virtio_p9_open(struct p9_dev *p9dev,
>> struct p9_pdu *pdu, u32 *outlen)
>> {
>> @@ -440,7 +450,6 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
>> for (i = 0; i < nwname; i++) {
>> struct stat st;
>> char tmp[PATH_MAX] = {0};
>> - char full_path[PATH_MAX];
>> char *str;
>> int ret;
>>
>> @@ -455,12 +464,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
>>
>> free(str);
>>
>> - if (rel_to_abs(p9dev, tmp, full_path, sizeof(full_path)) != 0) {
>> - errno = ENAMETOOLONG;
>> - goto err_out;
>> - }
>> -
>> - if (lstat(full_path, &st) < 0)
>> + if (stat_rel(p9dev, tmp, &st) != 0)
>> goto err_out;
>>
>> stat2qid(&st, &wqid);
>> @@ -614,7 +618,6 @@ static void virtio_p9_readdir(struct p9_dev *p9dev,
>> struct stat st;
>> struct p9_fid *fid;
>> struct dirent *dent;
>> - char full_path[PATH_MAX];
>> u64 offset, old_offset;
>>
>> rcount = 0;
>> @@ -645,11 +648,8 @@ static void virtio_p9_readdir(struct p9_dev *p9dev,
>> break;
>> }
>> old_offset = dent->d_off;
>> - if (rel_to_abs(p9dev, dent->d_name, full_path, sizeof(full_path)) != 0) {
>> - errno = ENAMETOOLONG;
>> - goto err_out;
>> - }
>> - lstat(full_path, &st);
>> + if (stat_rel(p9dev, dent->d_name, &st) != 0)
>> + memset(&st, -1, sizeof(st));
>
> Why the memset, and not goto err_out?
>
Because the user may not be allowed to stat some entries in a directory
and it shouldn't make readdir() fail.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
