Re: securityfs_create_dir strange comment
Hi Greg, >> >Try this instead: >> >if (!de) >> >return -ENOMEM; >> >if ((IS_ERR(de)) && (PTR_ERR(de) != -ENODEV)) >> >return PTR_ERR(de); >> >return 0; >> > >> >That should cover everything properly, right? >> >> In case memory could not be allocated, why does not securityfs_*() return >> ERR_PTR(-ENOMEM) then? (I think, that's the quintessential question after >> all. And thanks for giving an example what to do in the ENODEV case.) > >Actually, in reading the code (which might have helped in the first >place), we can never return NULL if securityfs is enabled. So we're back to the confusing comment ;-) >So you can just drop that first check entirely. > >Which makes me wonder, it might be easier to just return NULL if >securityfs is not enabled in the kernel, as long as no one checks that >improperly... I have actually had a look into the tree who even uses securityfs. The most prominent case are LSMs. They need CONFIG_SECURITY=y anyway, so securityfs is always enabled for those. What remains seems to be tpm_bios.c. Jan -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
On Wed, Feb 21, 2007 at 06:07:56PM +0100, Jan Engelhardt wrote: > Hello Greg, > > > On Feb 20 2007 20:05, Greg KH wrote: > > > >Try this instead: > > if (!de) > > return -ENOMEM; > > if ((IS_ERR(de)) && (PTR_ERR(de) != -ENODEV)) > > return PTR_ERR(de); > > return 0; > > > >That should cover everything properly, right? > > In case memory could not be allocated, why does not securityfs_*() return > ERR_PTR(-ENOMEM) then? (I think, that's the quintessential question after > all. And thanks for giving an example what to do in the ENODEV case.) Actually, in reading the code (which might have helped in the first place), we can never return NULL if securityfs is enabled. So you can just drop that first check entirely. Which makes me wonder, it might be easier to just return NULL if securityfs is not enabled in the kernel, as long as no one checks that improperly... Hope this helps, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
Hello Greg, On Feb 20 2007 20:05, Greg KH wrote: > >Try this instead: > if (!de) > return -ENOMEM; > if ((IS_ERR(de)) && (PTR_ERR(de) != -ENODEV)) > return PTR_ERR(de); > return 0; > >That should cover everything properly, right? In case memory could not be allocated, why does not securityfs_*() return ERR_PTR(-ENOMEM) then? (I think, that's the quintessential question after all. And thanks for giving an example what to do in the ENODEV case.) Jan -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
Hello Greg, On Feb 20 2007 20:05, Greg KH wrote: Try this instead: if (!de) return -ENOMEM; if ((IS_ERR(de)) (PTR_ERR(de) != -ENODEV)) return PTR_ERR(de); return 0; That should cover everything properly, right? In case memory could not be allocated, why does not securityfs_*() return ERR_PTR(-ENOMEM) then? (I think, that's the quintessential question after all. And thanks for giving an example what to do in the ENODEV case.) Jan -- - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
On Wed, Feb 21, 2007 at 06:07:56PM +0100, Jan Engelhardt wrote: Hello Greg, On Feb 20 2007 20:05, Greg KH wrote: Try this instead: if (!de) return -ENOMEM; if ((IS_ERR(de)) (PTR_ERR(de) != -ENODEV)) return PTR_ERR(de); return 0; That should cover everything properly, right? In case memory could not be allocated, why does not securityfs_*() return ERR_PTR(-ENOMEM) then? (I think, that's the quintessential question after all. And thanks for giving an example what to do in the ENODEV case.) Actually, in reading the code (which might have helped in the first place), we can never return NULL if securityfs is enabled. So you can just drop that first check entirely. Which makes me wonder, it might be easier to just return NULL if securityfs is not enabled in the kernel, as long as no one checks that improperly... Hope this helps, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
Hi Greg, Try this instead: if (!de) return -ENOMEM; if ((IS_ERR(de)) (PTR_ERR(de) != -ENODEV)) return PTR_ERR(de); return 0; That should cover everything properly, right? In case memory could not be allocated, why does not securityfs_*() return ERR_PTR(-ENOMEM) then? (I think, that's the quintessential question after all. And thanks for giving an example what to do in the ENODEV case.) Actually, in reading the code (which might have helped in the first place), we can never return NULL if securityfs is enabled. So we're back to the confusing comment ;-) So you can just drop that first check entirely. Which makes me wonder, it might be easier to just return NULL if securityfs is not enabled in the kernel, as long as no one checks that improperly... I have actually had a look into the tree who even uses securityfs. The most prominent case are LSMs. They need CONFIG_SECURITY=y anyway, so securityfs is always enabled for those. What remains seems to be tpm_bios.c. Jan -- - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
On Wed, Feb 21, 2007 at 12:45:40AM +0100, Jan Engelhardt wrote:
>
> On Feb 20 2007 14:26, Greg KH wrote:
> >On Tue, Feb 20, 2007 at 03:18:49PM -0600, Serge E. Hallyn wrote:
> >> Quoting Jan Engelhardt ([EMAIL PROTECTED]):
> >> > Hello list,
> >> >
> >> >
> >> > in security/inode.c, the comment for securityfs_create_dir() reads:
> >> >
> >> > If securityfs is not enabled in the kernel, the value -ENODEV
> >> > will be returned. It is not wise to check for this value, but
> >> > rather, check for NULL or !NULL instead as to eliminate the need
> >> > for #ifdef in the calling code.
> >> >
> >> > What is the actual callee that can return NULL - and what should
> >> > module_init() of a module return when securityfs_create_dir() returns
> >> > NULL?
> >>
> >> Hmm, this came from GregKH. It does seem based on the code that
> >> checking for -ENODEV is necessary, so I don't understand the comment.
> >
> >If securityfs_create_dir() returns NULL, then something bad happened and
> >your code needs to properly recover from it.
> >
> >Other than that, I don't understand the issue here.
>
> Consider:
>
> static __init int mymodule_init(void)
> {
> struct dentry *de;
> de = securityfs_create_dir("foobar", NULL);
>
> /* case 1 */
> if(IS_ERR(de))
> return PTR_ERR(de);
>
> /* case 2 */
> if(de == NULL)
> return WHAT_HERE; /* -EIO? */
> }
>
> There are two error cases. One: when the function gives us an error code.
> Two: When it returns NULL, without an error code. This looks bogus to me.
> What error is it, when there is no error? - And what should I return to
> modprobe in that case?
Try this instead:
if (!de)
return -ENOMEM;
if ((IS_ERR(de)) && (PTR_ERR(de) != -ENODEV))
return PTR_ERR(de);
return 0;
That should cover everything properly, right?
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
On Feb 20 2007 14:26, Greg KH wrote:
>On Tue, Feb 20, 2007 at 03:18:49PM -0600, Serge E. Hallyn wrote:
>> Quoting Jan Engelhardt ([EMAIL PROTECTED]):
>> > Hello list,
>> >
>> >
>> > in security/inode.c, the comment for securityfs_create_dir() reads:
>> >
>> >If securityfs is not enabled in the kernel, the value -ENODEV
>> >will be returned. It is not wise to check for this value, but
>> >rather, check for NULL or !NULL instead as to eliminate the need
>> >for #ifdef in the calling code.
>> >
>> > What is the actual callee that can return NULL - and what should
>> > module_init() of a module return when securityfs_create_dir() returns
>> > NULL?
>>
>> Hmm, this came from GregKH. It does seem based on the code that
>> checking for -ENODEV is necessary, so I don't understand the comment.
>
>If securityfs_create_dir() returns NULL, then something bad happened and
>your code needs to properly recover from it.
>
>Other than that, I don't understand the issue here.
Consider:
static __init int mymodule_init(void)
{
struct dentry *de;
de = securityfs_create_dir("foobar", NULL);
/* case 1 */
if(IS_ERR(de))
return PTR_ERR(de);
/* case 2 */
if(de == NULL)
return WHAT_HERE; /* -EIO? */
}
There are two error cases. One: when the function gives us an error code.
Two: When it returns NULL, without an error code. This looks bogus to me.
What error is it, when there is no error? - And what should I return to
modprobe in that case?
Jan
--
ft: http://freshmeat.net/p/chaostables/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
On Tue, Feb 20, 2007 at 03:18:49PM -0600, Serge E. Hallyn wrote: > Quoting Jan Engelhardt ([EMAIL PROTECTED]): > > Hello list, > > > > > > in security/inode.c, the comment for securityfs_create_dir() reads: > > > > If securityfs is not enabled in the kernel, the value -ENODEV > > will be returned. It is not wise to check for this value, but > > rather, check for NULL or !NULL instead as to eliminate the need > > for #ifdef in the calling code. > > > > What is the actual callee that can return NULL - and what should > > module_init() of a module return when securityfs_create_dir() returns > > NULL? > > Hmm, this came from GregKH. It does seem based on the code that > checking for -ENODEV is necessary, so I don't understand the comment. If securityfs_create_dir() returns NULL, then something bad happened and your code needs to properly recover from it. Other than that, I don't understand the issue here. confused, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
Quoting Jan Engelhardt ([EMAIL PROTECTED]): > Hello list, > > > in security/inode.c, the comment for securityfs_create_dir() reads: > > If securityfs is not enabled in the kernel, the value -ENODEV > will be returned. It is not wise to check for this value, but > rather, check for NULL or !NULL instead as to eliminate the need > for #ifdef in the calling code. > > What is the actual callee that can return NULL - and what should > module_init() of a module return when securityfs_create_dir() returns > NULL? Hmm, this came from GregKH. It does seem based on the code that checking for -ENODEV is necessary, so I don't understand the comment. thanks, -serge - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
Quoting Jan Engelhardt ([EMAIL PROTECTED]): Hello list, in security/inode.c, the comment for securityfs_create_dir() reads: If securityfs is not enabled in the kernel, the value -ENODEV will be returned. It is not wise to check for this value, but rather, check for NULL or !NULL instead as to eliminate the need for #ifdef in the calling code. What is the actual callee that can return NULL - and what should module_init() of a module return when securityfs_create_dir() returns NULL? Hmm, this came from GregKH. It does seem based on the code that checking for -ENODEV is necessary, so I don't understand the comment. thanks, -serge - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
On Tue, Feb 20, 2007 at 03:18:49PM -0600, Serge E. Hallyn wrote: Quoting Jan Engelhardt ([EMAIL PROTECTED]): Hello list, in security/inode.c, the comment for securityfs_create_dir() reads: If securityfs is not enabled in the kernel, the value -ENODEV will be returned. It is not wise to check for this value, but rather, check for NULL or !NULL instead as to eliminate the need for #ifdef in the calling code. What is the actual callee that can return NULL - and what should module_init() of a module return when securityfs_create_dir() returns NULL? Hmm, this came from GregKH. It does seem based on the code that checking for -ENODEV is necessary, so I don't understand the comment. If securityfs_create_dir() returns NULL, then something bad happened and your code needs to properly recover from it. Other than that, I don't understand the issue here. confused, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
On Feb 20 2007 14:26, Greg KH wrote:
On Tue, Feb 20, 2007 at 03:18:49PM -0600, Serge E. Hallyn wrote:
Quoting Jan Engelhardt ([EMAIL PROTECTED]):
Hello list,
in security/inode.c, the comment for securityfs_create_dir() reads:
If securityfs is not enabled in the kernel, the value -ENODEV
will be returned. It is not wise to check for this value, but
rather, check for NULL or !NULL instead as to eliminate the need
for #ifdef in the calling code.
What is the actual callee that can return NULL - and what should
module_init() of a module return when securityfs_create_dir() returns
NULL?
Hmm, this came from GregKH. It does seem based on the code that
checking for -ENODEV is necessary, so I don't understand the comment.
If securityfs_create_dir() returns NULL, then something bad happened and
your code needs to properly recover from it.
Other than that, I don't understand the issue here.
Consider:
static __init int mymodule_init(void)
{
struct dentry *de;
de = securityfs_create_dir(foobar, NULL);
/* case 1 */
if(IS_ERR(de))
return PTR_ERR(de);
/* case 2 */
if(de == NULL)
return WHAT_HERE; /* -EIO? */
}
There are two error cases. One: when the function gives us an error code.
Two: When it returns NULL, without an error code. This looks bogus to me.
What error is it, when there is no error? - And what should I return to
modprobe in that case?
Jan
--
ft: http://freshmeat.net/p/chaostables/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: securityfs_create_dir strange comment
On Wed, Feb 21, 2007 at 12:45:40AM +0100, Jan Engelhardt wrote:
On Feb 20 2007 14:26, Greg KH wrote:
On Tue, Feb 20, 2007 at 03:18:49PM -0600, Serge E. Hallyn wrote:
Quoting Jan Engelhardt ([EMAIL PROTECTED]):
Hello list,
in security/inode.c, the comment for securityfs_create_dir() reads:
If securityfs is not enabled in the kernel, the value -ENODEV
will be returned. It is not wise to check for this value, but
rather, check for NULL or !NULL instead as to eliminate the need
for #ifdef in the calling code.
What is the actual callee that can return NULL - and what should
module_init() of a module return when securityfs_create_dir() returns
NULL?
Hmm, this came from GregKH. It does seem based on the code that
checking for -ENODEV is necessary, so I don't understand the comment.
If securityfs_create_dir() returns NULL, then something bad happened and
your code needs to properly recover from it.
Other than that, I don't understand the issue here.
Consider:
static __init int mymodule_init(void)
{
struct dentry *de;
de = securityfs_create_dir(foobar, NULL);
/* case 1 */
if(IS_ERR(de))
return PTR_ERR(de);
/* case 2 */
if(de == NULL)
return WHAT_HERE; /* -EIO? */
}
There are two error cases. One: when the function gives us an error code.
Two: When it returns NULL, without an error code. This looks bogus to me.
What error is it, when there is no error? - And what should I return to
modprobe in that case?
Try this instead:
if (!de)
return -ENOMEM;
if ((IS_ERR(de)) (PTR_ERR(de) != -ENODEV))
return PTR_ERR(de);
return 0;
That should cover everything properly, right?
thanks,
greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
