On Sünndag 06 November 2005 13:05, OGAWA Hirofumi wrote:
> Arnd Bergmann <[email protected]> writes:
>
> > +#ifdef CONFIG_COMPAT
> > +/* vfat */
> > +#define VFAT_IOCTL_READDIR_BOTH32 _IOR('r', 1, struct compat_dirent[2])
> > +#define VFAT_IOCTL_READDIR_SHORT32 _IOR('r', 2, struct compat_dirent[2])
> ^^^^^^
>
> Please use a SPACE after #define, and kill a useless "/* vfat */".
ok.
> > +static long
> > +put_dirent32(struct dirent *d, struct compat_dirent __user * d32)
> > +{
> > + if (!access_ok(VERIFY_WRITE, d32, sizeof(struct compat_dirent)))
> > + return -EFAULT;
> > +
> > + __put_user(d->d_ino, &d32->d_ino);
> > + __put_user(d->d_off, &d32->d_off);
> > + __put_user(d->d_reclen, &d32->d_reclen);
> > + if (__copy_to_user(d32->d_name, d->d_name, d->d_reclen))
> > + return -EFAULT;
>
> Why don't we need to check the return value of __put_user()?
This is a bug in the code that I copied over, and it needs to be fixed,
thanks.
> > + set_fs(KERNEL_DS);
> > + ret = fat_dir_ioctl(file->f_dentry->d_inode, file, cmd, (unsigned long) &d);
> > + set_fs(oldfs);
> > + if (ret >= 0) {
> > + ret |= put_dirent32(&d[0], p);
> > + ret |= put_dirent32(&d[1], p + 1);
>
> If "ret" is not 0, we can't use "|=" here?
>
> This patch seems to have bugs, although original one too.
Right. In my patches I tried to change as little as possible to avoid
introducing new bugs, but this one is already so broken, that it's better
to do a clean implementation from scratch.
Arnd <><
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
