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?
> 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?
> + 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?
Will
--
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
