Re: user_memcpy()

"François Revol" <[email protected]> wrote:
> > François Revol made me recognize a design flaw in our user_memcpy()
> > /
> > user_strlcpy() API: you have to check the validity of the user 
> > address yourself, ie. you have to make sure it's in user space.
> > But we don't have a function for this, just the (kernel private) 
> > macro IS_USER_ADDRESS().
> Actually there is more to that...
[...]
> So you end up having is_valid_range(), which noone called anyway 
> (also 
> because it wasn't in a public header), and to which you didn't know 
> which flag to give anyway:

Doing is_valid_range() correctly is quite a bit overhead, and if 
possible I would like to get away without doing that :-)
I think we could introduce something like:
	bool called_from_kernel(void);

The VFS could set a flag in the current thread structure. For Haiku, we 
could also easily change the driver API (which we do anyway for new-
style drivers), but a common solution would be preferable, I guess.

> // from my sockfs header:
> 
> /*
> #define B_READ_AREA 1
> #define B_WRITE_AREA 2 */
> #define B_USER_READ_AREA    0x00000004  /* user can read */
> #define B_USER_WRITE_AREA    0x00000008  /* user can write */

Isn't it the other way around?
B_READ_AREA already expresses "user can read". What's missing is a 
B_KERNEL_READ_AREA (meaning: only kernel can read if used alone).

> /* why isn't that in KernelExport ??? */
> extern bool is_valid_range(void *start, size_t len, uint32 prot);

Probably out of stupidity - it's a bit too much for an oversight.

> Another problem of not being able to tell wether a call is made from 
> user or kernel side, is that you don't know what to use when checking 
> for uids and other perms. Of course it's not really of concerns for 
> now, but well mayeb it's time for it?
> Like, what should an fs addon call like creat() do when it's called 
> by 
> the kernel and it sees the current uid is not permitted to write in 
> wd?
> The kernel doesn't care about that, it's not what it asked the fs to 
> do. That is the kernel is always root, and should act as such.

I don't think it should be always root, even if it could simply switch 
its access rights if needed (which would only be safe, though, if these 
rights could be specified by thread).

> See:
> http://lxr.linux.no/source/include/asm-i386/uaccess.h

I don't really like this, though - the called_from_kernel() is a bit 
more flexible.

> Now, the elf loader in BeOS certainly doesn't care about those 
> sections, and the exception handler doesn't either. Probably Haiku 
> has/

What does this have to do with the ELF loader, anyway?

> work if we boot a new kernel that has say a new 1:3 vm split, or 
> maybe 
> a reverse split as it would make it easier for WINE.

We already did that for Haiku, btw.

> While Haiku's could be something closer to the linux one:
> status_t copy_to_user(void *to, void *from, size_t len)
> {
> 	if (to < current->addr_limit)
>         return EFAULT;
>     // set up exception handler for memcpy
>     return memcpy(to, from, len);
> }
> (but there is still the question of copying to an address in user 
> space 
> that is valid, but read only...)

That would still be catched by the Haiku version, at least (ie. it 
would safely fail).
BTW to be consistent to strlcpy_to_user() and user_memset(), I think I 
would like memcpy_to_user() more than copy_to_user().

> Anyway I'd propose 2 simple calls we could hardcode as macros for R5:
> 
> #if defined(COMPILE_FOR_R5) && !defined(B_ZETA_VERSION_VENTURE)
> #define check_for_user() false
> #define check_for_user_addr() false
> #define check_for_user_fd() true
> #else
> // check for valid user addresses 
> extern bool check_for_user_addr(void);
> // check for valid user permissions (for fds and such)
> extern bool check_for_user_fd(void);
> #endif

Why FD?
Anyway, I think we should have those two:
bool called_from_kernel(void);
bool is_user_address(void);

(or also called_from_user() instead)

> I think I can stick something in Zeta R1, but the time window is 
> shrinking, so it would be nice to reach a consensus there...

Would be nice, yes, although I we will definitely introduce a new 
driver architecture, anyway, so maybe the pressure to be compatible is 
not that high (even if we both could benefit from it).

Bye,
   Axel.



-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.
Get your fingers limbered up and give it your best shot. 4 great events, 4
opportunities to win big! Highest score wins.NEC IT Guy Games. Play to
win an NEC 61 plasma display. Visit http://www.necitguy.com/?r