This is from the new mount infrastructure introduced by
David Howells in November 2018. It makes sense that there
may be a problem in SELinux as well, as the code was introduced
by the same developer at the same time for the same purpose.
This is from the new mount infrastructure introduced by
David Howells in November 2018. It makes sense that there
may be a problem in SELinux as well, as the code was introduced
by the same developer at the same time for the same purpose.
It's Sunday morning and perhaps my mind is not yet in a "hey, let's
look at VFS kernel code!" mindset, but I'm not convinced the problem
is the 'param->string = NULL' assignment in the LSM hooks. In both
the case of SELinux and Smack that code ends up returning either a 0
(Smack) or a 1 (SELinux) - that's a little odd in it's own way, but I
don't believe it is relevant here - either way these return values are
not equal to -ENOPARAM so we should end up returning early from
vfs_parse_fs_param before it calls down into legacy_parse_param():
Taken from https://elixir.bootlin.com/linux/latest/source/fs/fs_context.c#L109 :
ret = security_fs_context_parse_param(fc, param);
if (ret != -ENOPARAM)
/* Param belongs to the LSM or is disallowed by the LSM; so
* don't pass to the FS.
*/
return ret;
if (fc->ops->parse_param) {
ret = fc->ops->parse_param(fc, param);
if (ret != -ENOPARAM)
return ret;
}
This is from the new mount infrastructure introduced by
David Howells in November 2018. It makes sense that there
may be a problem in SELinux as well, as the code was introduced
by the same developer at the same time for the same purpose.
It's Sunday morning and perhaps my mind is not yet in a "hey, let's
look at VFS kernel code!" mindset, but I'm not convinced the problem
is the 'param->string = NULL' assignment in the LSM hooks. In both
the case of SELinux and Smack that code ends up returning either a 0
(Smack) or a 1 (SELinux) - that's a little odd in it's own way, but I
don't believe it is relevant here - either way these return values are
not equal to -ENOPARAM so we should end up returning early from
vfs_parse_fs_param before it calls down into legacy_parse_param():
Taken from https://elixir.bootlin.com/linux/latest/source/fs/fs_context.c#L109 :
ret = security_fs_context_parse_param(fc, param);
if (ret != -ENOPARAM)
/* Param belongs to the LSM or is disallowed by the LSM; so
* don't pass to the FS.
*/
return ret;
if (fc->ops->parse_param) {
ret = fc->ops->parse_param(fc, param);
if (ret != -ENOPARAM)
return ret;
}
Hi Paul,
You are right.
I almost connected the dots, but not exactly.
Now that I read more code around, setting "param->string = NULL" in
smack_fs_context_parse_param() looks correct to me (the fs copies and
takes ownership of the string).
I don't see how the crash happened...
This is from the new mount infrastructure introduced by
David Howells in November 2018. It makes sense that there
may be a problem in SELinux as well, as the code was introduced
by the same developer at the same time for the same purpose.
It's Sunday morning and perhaps my mind is not yet in a "hey, let's
look at VFS kernel code!" mindset, but I'm not convinced the problem
is the 'param->string = NULL' assignment in the LSM hooks. In both
the case of SELinux and Smack that code ends up returning either a 0
(Smack) or a 1 (SELinux) - that's a little odd in it's own way, but I
don't believe it is relevant here - either way these return values are
not equal to -ENOPARAM so we should end up returning early from
vfs_parse_fs_param before it calls down into legacy_parse_param():
Taken from https://elixir.bootlin.com/linux/latest/source/fs/fs_context.c#L109 :
ret = security_fs_context_parse_param(fc, param);
if (ret != -ENOPARAM)
/* Param belongs to the LSM or is disallowed by the LSM; so
* don't pass to the FS.
*/
return ret;
if (fc->ops->parse_param) {
ret = fc->ops->parse_param(fc, param);
if (ret != -ENOPARAM)
return ret;
}
Hi Paul,
You are right.
I almost connected the dots, but not exactly.
Now that I read more code around, setting "param->string = NULL" in
smack_fs_context_parse_param() looks correct to me (the fs copies and
takes ownership of the string).
I don't see how the crash happened...
FWIW, I poked around a bit too and couldn't see anything obvious
either, but I can't pretend to know as much about the VFS layer as the
VFS folks. Hopefully they might have better luck.
--
paul moore
www.paul-moore.com
This is from the new mount infrastructure introduced by
David Howells in November 2018. It makes sense that there
may be a problem in SELinux as well, as the code was introduced
by the same developer at the same time for the same purpose.
It's Sunday morning and perhaps my mind is not yet in a "hey, let's
look at VFS kernel code!" mindset, but I'm not convinced the problem
is the 'param->string = NULL' assignment in the LSM hooks. In both
the case of SELinux and Smack that code ends up returning either a 0
(Smack) or a 1 (SELinux) - that's a little odd in it's own way, but I
don't believe it is relevant here - either way these return values are
not equal to -ENOPARAM so we should end up returning early from
vfs_parse_fs_param before it calls down into legacy_parse_param():
Taken from https://elixir.bootlin.com/linux/latest/source/fs/fs_context.c#L109 :
ret = security_fs_context_parse_param(fc, param);
if (ret != -ENOPARAM)
/* Param belongs to the LSM or is disallowed by the LSM; so
* don't pass to the FS.
*/
return ret;
if (fc->ops->parse_param) {
ret = fc->ops->parse_param(fc, param);
if (ret != -ENOPARAM)
return ret;
}
Hi Paul,
You are right.
I almost connected the dots, but not exactly.
Now that I read more code around, setting "param->string = NULL" in
smack_fs_context_parse_param() looks correct to me (the fs copies and
takes ownership of the string).
I don't see how the crash happened...
FWIW, I poked around a bit too and couldn't see anything obvious
either, but I can't pretend to know as much about the VFS layer as the
VFS folks. Hopefully they might have better luck.
I'm not sure that's right.
If the smack hook runs first, it will set
param->string = NULL
now the selinux hook runs. But the selinux param hook doesn't end up in
selinux_add_opt() instead it will fail before
opt = fs_parse(fc, selinux_fs_parameters, param, &result);
which will return -ENOPARAM since it's not a selinux option subsequently
causing the crash.
Does that sound plausible?
Christian
This is from the new mount infrastructure introduced by
David Howells in November 2018. It makes sense that there
may be a problem in SELinux as well, as the code was introduced
by the same developer at the same time for the same purpose.
It's Sunday morning and perhaps my mind is not yet in a "hey, let's
look at VFS kernel code!" mindset, but I'm not convinced the problem
is the 'param->string = NULL' assignment in the LSM hooks. In both
the case of SELinux and Smack that code ends up returning either a 0
(Smack) or a 1 (SELinux) - that's a little odd in it's own way, but I
don't believe it is relevant here - either way these return values are
not equal to -ENOPARAM so we should end up returning early from
vfs_parse_fs_param before it calls down into legacy_parse_param():
Taken from https://elixir.bootlin.com/linux/latest/source/fs/fs_context.c#L109 :
ret = security_fs_context_parse_param(fc, param);
if (ret != -ENOPARAM)
/* Param belongs to the LSM or is disallowed by the LSM; so
* don't pass to the FS.
*/
return ret;
if (fc->ops->parse_param) {
ret = fc->ops->parse_param(fc, param);
if (ret != -ENOPARAM)
return ret;
}
Hi Paul,
You are right.
I almost connected the dots, but not exactly.
Now that I read more code around, setting "param->string = NULL" in
smack_fs_context_parse_param() looks correct to me (the fs copies and
takes ownership of the string).
I don't see how the crash happened...
FWIW, I poked around a bit too and couldn't see anything obvious
either, but I can't pretend to know as much about the VFS layer as the
VFS folks. Hopefully they might have better luck.
I'm not sure that's right.
If the smack hook runs first, it will set
param->string = NULL
now the selinux hook runs. But the selinux param hook doesn't end up in
selinux_add_opt() instead it will fail before
opt = fs_parse(fc, selinux_fs_parameters, param, &result);
which will return -ENOPARAM since it's not a selinux option subsequently
causing the crash.
Does that sound plausible?
No. You can't (currently) have both Smack and SELinux enabled at
the same time. If you're invoking both the Smack hook and the SELinux
hook you're doing somthing way wrong.
This is from the new mount infrastructure introduced by
David Howells in November 2018. It makes sense that there
may be a problem in SELinux as well, as the code was introduced
by the same developer at the same time for the same purpose.
It's Sunday morning and perhaps my mind is not yet in a "hey, let's
look at VFS kernel code!" mindset, but I'm not convinced the problem
is the 'param->string = NULL' assignment in the LSM hooks. In both
the case of SELinux and Smack that code ends up returning either a 0
(Smack) or a 1 (SELinux) - that's a little odd in it's own way, but I
don't believe it is relevant here - either way these return values are
not equal to -ENOPARAM so we should end up returning early from
vfs_parse_fs_param before it calls down into legacy_parse_param():
Taken from https://elixir.bootlin.com/linux/latest/source/fs/fs_context.c#L109 :
ret = security_fs_context_parse_param(fc, param);
if (ret != -ENOPARAM)
/* Param belongs to the LSM or is disallowed by the LSM; so
* don't pass to the FS.
*/
return ret;
if (fc->ops->parse_param) {
ret = fc->ops->parse_param(fc, param);
if (ret != -ENOPARAM)
return ret;
}
Hi Paul,
You are right.
I almost connected the dots, but not exactly.
Now that I read more code around, setting "param->string = NULL" in
smack_fs_context_parse_param() looks correct to me (the fs copies and
takes ownership of the string).
I don't see how the crash happened...
FWIW, I poked around a bit too and couldn't see anything obvious
either, but I can't pretend to know as much about the VFS layer as the
VFS folks. Hopefully they might have better luck.
I'm not sure that's right.
If the smack hook runs first, it will set
param->string = NULL
now the selinux hook runs. But the selinux param hook doesn't end up in
selinux_add_opt() instead it will fail before
opt = fs_parse(fc, selinux_fs_parameters, param, &result);
which will return -ENOPARAM since it's not a selinux option subsequently
causing the crash.
Does that sound plausible?
No. You can't (currently) have both Smack and SELinux enabled at
Ah, I thought that already worked. :)
I'm EOD here but I'll try to look closer tomorrow or after the weekend.
Christian