Re: Passing kernel space data buffer where userspace is expected.

["Prasad Joshi" <prasadjoshi124@xxxxxxxxx>]

On Wed, Aug 20, 2008 at 11:05 AM, Sachin Gaikwad <sachin.kernel@xxxxxxxxx> wrote:

Hi Prasad,

On Wed, Aug 6, 2008 at 6:50 PM, Prasad Joshi <prasadjoshi124@xxxxxxxxx> wrote:
> Hi All,
>
> I want to encrypt the data before it is written to the disk and decrypt is
> after it is being read from the disk. so copied the ext2 source code in my
> directory.
>
> Then modified the ext2 file_operations structure to invoke my read and write
> functions instead of do_sync_read/write
>
> const struct file_operations ext2_file_operations = {
>     .llseek     = generic_file_llseek,
>     .read       = my_read,                   /* do_sync_read  */
> <<<<<<<<<<<<<<<<<<<<<<<<
>     .write      = my_write,                  /* do_sync_write */
> <<<<<<<<<<<<<<<<<<<<<<<<
>     .aio_read   = generic_file_aio_read,
>     .aio_write  = generic_file_aio_write,
>     .unlocked_ioctl = ext2_ioctl,
> #ifdef CONFIG_COMPAT
>     .compat_ioctl   = ext2_compat_ioctl,
> #endif
>     .mmap       = generic_file_mmap,
>     .open       = generic_file_open,
>     .release    = ext2_release_file,
>     .fsync      = ext2_sync_file,
>     .splice_read    = generic_file_splice_read,
>     .splice_write   = generic_file_splice_write,
> };
>
>
> For time being i used simplest encryption XOR 5
> int encrypt_data(char *data, size_t len)
> {
>     int i;
>     for(i=0; i < len; i++)
>         data[i] ^= 5;
>
>     return 0;
> }
>
> int decrypt_data(char *data, size_t len)
> {
>     int i;
>     for(i=0; i < len; i++)
>         data[i] ^= 5;
>
>     return 0;
> }
>
> ssize_t my_read(struct file *filp, char __user *buf, size_t len, loff_t
> *ppos)
> {
>     size_t ret;
>     char *data = "" GFP_KERNEL);
>
>     ret = do_sync_read(filp, data, len, ppos);
>     decrypt_data(data, len);
>     copy_to_user(buf, data, len);
>
>     kfree(data);
>     return ret;
> }
>
> In write I am supposed to encrypt the data before it is written to the disk,
> ie encrypt the data before it is passed to the do_sync_write.
> But, the data is in the buf which is passed from the user process and has
> the type             const char *buf       so      I can't modify the
> buf
>
> I thought of  copying data from the      (userland)   buf     to some
> variable in the kernelspace and then encrypt it and pass to do_sync_write()
>
> This is what I did
>
> ssize_t my_write(struct file *filp, const char __user *buf, size_t len,
> loff_t *ppos)
> {
>     size_t ret;
>     char *data = "" GFP_KERNEL);
>
>     copy_from_user(data, buf, len);
>     encrypt_data(data, len);
>     ret = do_sync_write(filp, data, len, ppos);
>
>     kfree(data);
>     return ret;
> }
>
> After running the code and writing data to the file using echo "test" >
> /mnt_pt/file    I am getting bad address error
>
> Is it because do_sync_write() also expects buf pointer from the userspace?
> ssize_t do_sync_write(struct file *filp, const char __user *buf, size_t len,
> loff_t *ppos)

You are right. Thats exactly the reason for this.

>
> But, the only thing do_sync_write should be concerned with whether it can
> access the data pointer or not. So, if the data pointer is valid and kernel
> is able to access the location why so worry about userspace pointer?

These are internl kernel functions(do_sync_read/write) and they expect
__user user space pointer. Often it has been said that __user is just
a documentation thing so that programmer is careful about copying it
to kernel space. And same is done when you pass such user space
pointer to these functions. For this case :

do_sync_read  -----> filp->f_op->aio_read -----> generic_file_aio_read
-----> generic_segment_checks ------> access_ok ------> __range_not_ok

If you closing look at __range_not_ok macro, it is checking is address
is with reach of current thread's segment limit (I am not good at
reading assembly code), which is not the case in your code as it is a
kernel pointer. Hence it says "bad address".

What you can do ?

ssize_t my_write(struct file *filp, const char __user *buf, size_t len,
  loff_t *ppos)
{
    size_t ret;

    mm_segment_t oldfs;

    char *data = "" GFP_KERNEL);

    copy_from_user(data, buf, len);
    encrypt_data(data, len);

    oldfs = get_fs();      /* Read comments in include/asm-x86/uaccess_64.h */
    set_fs (KERNEL_DS);

    ret = do_sync_write(filp, data, len, ppos);

    set_fs(oldfs);

Thanks Sachin, this worked correctly.
As you said, with set_fs(KERNEL_DS) we are changing the address limit of the process to 0-4G, which was previously 0-3G (USER_DS for user space). So, it is very important to reset the current thread segment type to whatever it was.

But, what if I failed/forgot to restore th.e segment type to USER_DS. So, now the process is now capable of accessing whole 4G address space.
What does the extra capability a process will have if it has access to whole 4G address space, instead of 0-3G?
How does this harm my system?

someone please explain.......

Thanks and Regards,
Prasad.

    kfree(data);
    return ret;
 }

I wonder why you got that error for just write and not read. You are
passing kernel pointer to do_sync_read also.

Hope this helps,
Sachin


>
> Thanks and Regards,
> Prasad
>