Hi,
I just noticed that mqs were included in the 2.6.6-rc1.
Looking over the code in libmqueue-4.31, I noticed the checks for the
name validity in the mq_open and mq_unlink. Why are they needed? They
are pointless if the code in kernel depends on the valid name, because
libmqueue can not be the only library using the mq interface (all the
libcs being the next candidates), and they are pointless in case the
kernel does not depend on them, because it will return an error anyway,
if that is defined by the implementation.
Can the checks be removed? Cutting of the first character will become
unconditional than, btw, which is also not good.
The other thing: mq_open gets the last two args (attr and mode)
unconditionally. What will the kernel code get in the arguments if
O_CREAT is not specified, and the calling code did not given the
arguments?
I don't think this will ever cause any problem, but the code is unclean
in this aspect.
Sincerely,
Alex Riesen
Alex Riesen wrote:
> Looking over the code in libmqueue-4.31, I noticed the checks for the
> name validity in the mq_open and mq_unlink. Why are they needed? They
> are pointless if the code in kernel depends on the valid name,
You are contradicting yourself.
Anyway, non-absolute path names passed to the functions mean the
behavior is unspecified. No portable application must ever do this. It
is enforced for this reason plus if there comes a time when we want to
do something special which doesn't conflict with standard-compliant
behavior we have a possibility for that. Unlike wh6at you think, the
tests *are* useful.
--
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
> > Looking over the code in libmqueue-4.31, I noticed the checks for the
> > name validity in the mq_open and mq_unlink. Why are they needed? They
> > are pointless if the code in kernel depends on the valid name,
>
> You are contradicting yourself.
>
> Anyway, non-absolute path names passed to the functions mean the
> behavior is unspecified. No portable application must ever do this. It
> is enforced for this reason plus if there comes a time when we want to
> do something special which doesn't conflict with standard-compliant
> behavior we have a possibility for that. Unlike wh6at you think, the
> tests *are* useful.
My concern is that the tests are rather pointing that something in
kernel is not implemented correctly. _The_ checks in particular.
Because if they _are_ implemented correctly, you don't need to patch the
functionality in the user space.
And if the kernel code does check the incoming arguments correctly,
what is the point to check them again? Just to make the point, that
passing in not an absolute path is not portable?
Alex Riesen wrote:
> And if the kernel code does check the incoming arguments correctly,
> what is the point to check them again? Just to make the point, that
> passing in not an absolute path is not portable?
Forget what the kernel does. This is enforcement of the API the runtime
provides. If must be stable regardless of what the kernel does.
Including kernel changes which allow special names which do funky,
non-standard things.
--
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
* Alex Riesen ([email protected]) wrote:
> My concern is that the tests are rather pointing that something in
> kernel is not implemented correctly. _The_ checks in particular.
> Because if they _are_ implemented correctly, you don't need to patch the
> functionality in the user space.
>
> And if the kernel code does check the incoming arguments correctly,
> what is the point to check them again? Just to make the point, that
> passing in not an absolute path is not portable?
The kernel interface is simple and clean. And in fact, requires no
slashes else you'll get -EACCES. It's not POSIX, but the library
interface is.
We just discussed this yesterday:
http://marc.theaimsgroup.com/?t=108205593100003&r=1&w=2
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
Chris Wright, Sat, Apr 17, 2004 00:22:17 +0200:
> > My concern is that the tests are rather pointing that something in
> > kernel is not implemented correctly. _The_ checks in particular.
> > Because if they _are_ implemented correctly, you don't need to patch the
> > functionality in the user space.
> >
> > And if the kernel code does check the incoming arguments correctly,
> > what is the point to check them again? Just to make the point, that
> > passing in not an absolute path is not portable?
>
> The kernel interface is simple and clean. And in fact, requires no
> slashes else you'll get -EACCES. It's not POSIX, but the library
> interface is.
>
> We just discussed this yesterday:
>
> http://marc.theaimsgroup.com/?t=108205593100003&r=1&w=2
now, what's is the check in the library for? BTW, it is returning the
other error code (EINVAL instead of EACCES), just on top of all the
confusion with slashes.
* Alex Riesen ([email protected]) wrote:
> Chris Wright, Sat, Apr 17, 2004 00:22:17 +0200:
> > The kernel interface is simple and clean. And in fact, requires no
> > slashes else you'll get -EACCES. It's not POSIX, but the library
> > interface is.
> >
> > We just discussed this yesterday:
> >
> > http://marc.theaimsgroup.com/?t=108205593100003&r=1&w=2
>
> now, what's is the check in the library for? BTW, it is returning the
> other error code (EINVAL instead of EACCES), just on top of all the
> confusion with slashes.
EINVAL in the library, sure. EACCES is if you directly use the kernel
interface and pass it a name with any slashes in it. The two interfaces
(library and kernel) aren't required to be identical. Kernel is kept
simplest w/ no slashes, library provides POSIX compliance.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
Chris Wright, Sat, Apr 17, 2004 01:56:21 +0200:
> > > The kernel interface is simple and clean. And in fact, requires no
> > > slashes else you'll get -EACCES. It's not POSIX, but the library
> > > interface is.
> > >
> > > We just discussed this yesterday:
> > >
> > > http://marc.theaimsgroup.com/?t=108205593100003&r=1&w=2
> >
> > now, what's is the check in the library for? BTW, it is returning the
> > other error code (EINVAL instead of EACCES), just on top of all the
> > confusion with slashes.
>
> EINVAL in the library, sure. EACCES is if you directly use the kernel
> interface and pass it a name with any slashes in it. The two interfaces
> (library and kernel) aren't required to be identical. Kernel is kept
> simplest w/ no slashes, library provides POSIX compliance.
>
Ok. It's just that every provider of the _kernel_ interface to user
space has now to take care of being posix-compliant. Write the code for
checks, iow. That is not the case for "open", for instance.
And besides, with the patch applied the kernel is also posix compliant,
isn't it? Noone still answered why the check in library in this case.
(The patch is not applied yet, btw)
Alex wrote:
>Ok. It's just that every provider of the _kernel_ interface to user
>space has now to take care of being posix-compliant. Write the code for
>checks, iow. That is not the case for "open", for instance.
>And besides, with the patch applied the kernel is also posix compliant,
>isn't it?
>
No. E.g. mq_notify(,&{.sigev_notify=SIGEV_THREAD) cannot be implemented
in kernel space. And sys_mq_getsetattr isn't posix compliant either -
the user space library must implement mq_getattr and mq_setattr on top
of the kernel API.
The kernel API was designed to be simple and flexible. Perhaps we want
to extend the kernel implementation in the future, and then a leading
slash could be used to indicate that we are using the new features.
--
Manfred
Manfred Spraul, Sat, Apr 17, 2004 12:49:08 +0200:
> Alex wrote:
>
> >Ok. It's just that every provider of the _kernel_ interface to user
> >space has now to take care of being posix-compliant. Write the code for
> >checks, iow. That is not the case for "open", for instance.
> >And besides, with the patch applied the kernel is also posix compliant,
> >isn't it?
> >
> No. E.g. mq_notify(,&{.sigev_notify=SIGEV_THREAD) cannot be implemented
> in kernel space. And sys_mq_getsetattr isn't posix compliant either -
> the user space library must implement mq_getattr and mq_setattr on top
> of the kernel API.
Ok. It's inevitable, as it looks.
> The kernel API was designed to be simple and flexible. Perhaps we want
> to extend the kernel implementation in the future, and then a leading
> slash could be used to indicate that we are using the new features.
and userspace will need to be updated (the slashes are cut off in libmqueue)