On Fri, 19 Feb 2010 11:12:26 +0530
"Aneesh Kumar K.V" <aneesh.kumar@xxxxxxxxxxxxxxxxxx> wrote:
> The below set of patches implement open by handle support using exportfs
> operations.
I have a couple of questions...starting with: what is the use case for
this functionality? There must, clearly, be some kind of application
which needs to be able to open by file handle, but I'm not sure what
that would be.
I agree with Andreas that the handle length looks like a bit of a
crapshoot. How should an application developer know how much memory to
dedicate to this?
In do_sys_name_to_handle() you have:
> + f_handle = kmalloc(handle_size, GFP_KERNEL);
handle_size comes directly from user space. Perhaps it should be
sanity-checked? (I would also just use handle->handle_size; later
handle_size contains the *real* handle size, and I found that confusing.
Yes, I'm easily confused, but...)
> + handle->handle_type = retval;
> + if (handle_size < handle->handle_size) {
> + if (copy_to_user(handle->f_handle, f_handle,
> + handle_size*sizeof(u32)))
> + retval = -EFAULT;
> + retval = 0;
> + } else
> + retval = -EAGAIN;
> + handle->handle_size = handle_size;
EAGAIN seems like a strange thing to return here. ENOSPC maybe?
Are you missing an "else" before the "retval = 0;" line? You'll never
return -EFAULT here.
In do_sys_open_by_handle():
> + if (!capable(CAP_SYS_ADMIN))
> + /* Allow open by handle only by sysadmin */
> + return -EPERM;
I assume this is to avoid access to readable files within unreadable
directories? Otherwise you could check the permissions of the target
dentry.
CAP_SYS_ADMIN seems like the wrong capability, though. CAP_DAC_OVERRIDE
might make more sense?
Is there any sense in allowing anybody to call name_to_handle() if
open_by_handle() is restricted?
Thanks,
jon
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
