Re: [Vserver] sendfile crash? (2.6.13.4 with vserver 2.1.0-rc4)

From: Grzegorz Nosek <grzegorz.nosek_at_gmail.com>
Date: Thu 03 Nov 2005 - 20:01:14 GMT
Message-ID: <121a28810511031201q612cdf6dx@mail.gmail.com>

2005/11/3, Herbert Poetzl <herbert@13thfloor.at>:
> On Thu, Nov 03, 2005 at 05:38:43PM +0100, Grzegorz Nosek wrote:
> > Hello all
> >
> > I needed to apply the patch below in order to keep the kernel from
> > oopsing (in some older revisions) or freezing solid (in the newest,
> > listed in the subject.
>
> as follow up, please try the following patch instead:
>
> http://lkml.org/lkml/diff/2005/11/3/161/1
>
> rationale: http://lkml.org/lkml/2005/11/3/161
>
> HTH,
> Herbert
>
>

Hello,

I think I disagree that my proposed patch is hiding the real bug
because the ppos and max variables aren't ever used in do_sendfile.
They're blindly passed to vfs_sendfile, which does the checks and
returns -EOVERFLOW if needed.

IMHO my patch is more of The Right Way (tm) because it doesn't involve
modifications in another function (only do_sendfile is affected, which
is modified heavily anyway).

Of course, you're free to either include it or ignore it.

(my definitions of vfs_sendfile and do_sendfile with some random
annotations below)

ssize_t vfs_sendfile(struct file *out_file, struct file *in_file, loff_t *ppos,
                     size_t count, loff_t max)
{
        struct inode * in_inode, * out_inode;
        loff_t pos;
        ssize_t ret;

        /* verify in_file */
        in_inode = in_file->f_dentry->d_inode;
        if (!in_inode)
                return -EINVAL;
        if (!in_file->f_op || !in_file->f_op->sendfile)
                return -EINVAL;

// !!! ppos is validated here
        if (!ppos)
                ppos = &in_file->f_pos;
        else
                if (!(in_file->f_mode & FMODE_PREAD))
                        return -ESPIPE;

        ret = rw_verify_area(FLOCK_VERIFY_READ, in_file, ppos, count);
        if (ret)
                return ret;

        /* verify out_file */
        out_inode = out_file->f_dentry->d_inode;
        if (!out_inode)
                return -EINVAL;
        if (!out_file->f_op || !out_file->f_op->sendpage)
                return -EINVAL;

        ret = rw_verify_area(FLOCK_VERIFY_WRITE, out_file,
&out_file->f_pos, count);
        if (ret)
                return ret;

        ret = security_file_permission (out_file, MAY_WRITE);
        if (ret)
                return ret;

// !!! max is validated here
        if (!max)
                max = min(in_inode->i_sb->s_maxbytes,
out_inode->i_sb->s_maxbytes);

        pos = *ppos;
        if (unlikely(pos < 0))
                return -EINVAL;
        if (unlikely(pos + count > max)) {
                if (pos >= max)
                        return -EOVERFLOW;
                count = max - pos;
        }

        ret = in_file->f_op->sendfile(in_file, ppos, count,
file_send_actor, out_file);

        if (*ppos > max)
                return -EOVERFLOW;
        return ret;
}

EXPORT_SYMBOL(vfs_sendfile);

static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos,
                           size_t count, loff_t max)
{
        struct file * in_file, * out_file;
        ssize_t retval;
        int fput_needed_in, fput_needed_out;

        /*
         * Get input file, and verify that it is ok..
         */
        retval = -EBADF;
        in_file = fget_light(in_fd, &fput_needed_in);
        if (!in_file)
                goto out;
        if (!(in_file->f_mode & FMODE_READ))
                goto fput_in;

        retval = security_file_permission (in_file, MAY_READ);
        if (retval)
                goto fput_in;

        /*
         * Get output file, and verify that it is ok..
         */
        retval = -EBADF;
        out_file = fget_light(out_fd, &fput_needed_out);
        if (!out_file)
                goto fput_in;
        if (!(out_file->f_mode & FMODE_WRITE))
                goto fput_out;

        retval = vfs_sendfile(out_file, in_file, ppos, count, max);

// !!! if vfs_sendfile returned -EOVERFLOW, it propagates out of
do_sendfile too (and doesn't skip fput_XXX)

        if (retval > 0) {
                current->rchar += retval;
                current->wchar += retval;
        }
        current->syscr++;
        current->syscw++;

fput_out:
        fput_light(out_file, fput_needed_out);
fput_in:
        fput_light(in_file, fput_needed_in);
out:
        return retval;
}
_______________________________________________
Vserver mailing list
Vserver@list.linux-vserver.org
http://list.linux-vserver.org/mailman/listinfo/vserver
Received on Thu Nov 3 20:01:34 2005

[Next/Previous Months] [Main vserver Project Homepage] [Howto Subscribe/Unsubscribe] [Paul Sladen's vserver stuff]
Generated on Thu 03 Nov 2005 - 20:01:37 GMT by hypermail 2.1.8