On 11/17/2016 01:20 PM, Will Deacon wrote:
> On Thu, Nov 10, 2016 at 04:18:54PM +0100, G. Campana wrote:
>> On 08/11/2016 03:38, Will Deacon wrote:
>>> On Tue, Oct 18, 2016 at 06:03:05PM +0200, G. Campana wrote:
>>>> @@ -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.
>
> Ok, but is memsetting to -1 really the right thing to do? This gets
> "converted" into a p9_qid_t, which will then look pretty strange (path
> and version will be set to 0xff, type will be set to P9_QTDIR).
>
Before this patch, st was either uninitialized or invalid if lstat
failed, hence the memset call which doesn't break the logic of this
function. I only tried to fix vulnerabilities in this patch series, and
I think this issue deserves a separate patch. What do you think?
> Does 9p not have a better way to communicate that the stat failed?
>
> 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
