From: Roberto Sassu roberto.sassu@huawei.com
Commit ac4e97abce9b8 ("scatterlist: sg_set_buf() argument must be in linear mapping") requires that both the signature and the digest resides in the linear mapping area.
However, more recently commit ba14a194a434c ("fork: Add generic vmalloced stack support"), made it possible to move the stack in the vmalloc area, which could make the requirement of the first commit not satisfied anymore.
If CONFIG_SG=y and CONFIG_VMAP_STACK=y, the following BUG() is triggered:
[ 467.077359] kernel BUG at include/linux/scatterlist.h:163! [ 467.077939] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[...]
[ 467.095225] Call Trace: [ 467.096088] <TASK> [ 467.096928] ? rcu_read_lock_held_common+0xe/0x50 [ 467.097569] ? rcu_read_lock_sched_held+0x13/0x70 [ 467.098123] ? trace_hardirqs_on+0x2c/0xd0 [ 467.098647] ? public_key_verify_signature+0x470/0x470 [ 467.099237] asymmetric_verify+0x14c/0x300 [ 467.099869] evm_verify_hmac+0x245/0x360 [ 467.100391] evm_inode_setattr+0x43/0x190
The failure happens only for the digest, as the pointer comes from the stack, and not for the signature, which instead was allocated by vfs_getxattr_alloc().
Fix this by making a copy of both in asymmetric_verify(), so that the linear mapping requirement is always satisfied, regardless of the caller.
Cc: stable@vger.kernel.org # 4.9.x Fixes: ba14a194a434 ("fork: Add generic vmalloced stack support") Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/digsig_asymmetric.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c index 895f4b9ce8c6..635238d5c7fe 100644 --- a/security/integrity/digsig_asymmetric.c +++ b/security/integrity/digsig_asymmetric.c @@ -122,11 +122,26 @@ int asymmetric_verify(struct key *keyring, const char *sig, goto out; }
- pks.digest = (u8 *)data; + pks.digest = kmemdup(data, datalen, GFP_KERNEL); + if (!pks.digest) { + ret = -ENOMEM; + goto out; + } + pks.digest_size = datalen; - pks.s = hdr->sig; + + pks.s = kmemdup(hdr->sig, siglen, GFP_KERNEL); + if (!pks.s) { + kfree(pks.digest); + ret = -ENOMEM; + goto out; + } + pks.s_size = siglen; + ret = verify_signature(key, &pks); + kfree(pks.digest); + kfree(pks.s); out: key_put(key); pr_debug("%s() = %d\n", __func__, ret);
On Fri, 2022-11-04 at 13:20 +0100, Roberto Sassu wrote:
From: Roberto Sassu roberto.sassu@huawei.com
Commit ac4e97abce9b8 ("scatterlist: sg_set_buf() argument must be in linear mapping") requires that both the signature and the digest resides in the linear mapping area.
However, more recently commit ba14a194a434c ("fork: Add generic vmalloced stack support"), made it possible to move the stack in the vmalloc area, which could make the requirement of the first commit not satisfied anymore.
If CONFIG_SG=y and CONFIG_VMAP_STACK=y, the following BUG() is triggered:
Hi Mimi
did you have the chance to have a look at this patch?
Thanks
Roberto
[ 467.077359] kernel BUG at include/linux/scatterlist.h:163! [ 467.077939] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[...]
[ 467.095225] Call Trace: [ 467.096088] <TASK> [ 467.096928] ? rcu_read_lock_held_common+0xe/0x50 [ 467.097569] ? rcu_read_lock_sched_held+0x13/0x70 [ 467.098123] ? trace_hardirqs_on+0x2c/0xd0 [ 467.098647] ? public_key_verify_signature+0x470/0x470 [ 467.099237] asymmetric_verify+0x14c/0x300 [ 467.099869] evm_verify_hmac+0x245/0x360 [ 467.100391] evm_inode_setattr+0x43/0x190
The failure happens only for the digest, as the pointer comes from the stack, and not for the signature, which instead was allocated by vfs_getxattr_alloc().
Fix this by making a copy of both in asymmetric_verify(), so that the linear mapping requirement is always satisfied, regardless of the caller.
Cc: stable@vger.kernel.org # 4.9.x Fixes: ba14a194a434 ("fork: Add generic vmalloced stack support") Signed-off-by: Roberto Sassu roberto.sassu@huawei.com
security/integrity/digsig_asymmetric.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c index 895f4b9ce8c6..635238d5c7fe 100644 --- a/security/integrity/digsig_asymmetric.c +++ b/security/integrity/digsig_asymmetric.c @@ -122,11 +122,26 @@ int asymmetric_verify(struct key *keyring, const char *sig, goto out; }
- pks.digest = (u8 *)data;
- pks.digest = kmemdup(data, datalen, GFP_KERNEL);
- if (!pks.digest) {
ret = -ENOMEM;goto out;- }
- pks.digest_size = datalen;
- pks.s = hdr->sig;
- pks.s = kmemdup(hdr->sig, siglen, GFP_KERNEL);
- if (!pks.s) {
kfree(pks.digest);ret = -ENOMEM;goto out;- }
- pks.s_size = siglen;
- ret = verify_signature(key, &pks);
- kfree(pks.digest);
- kfree(pks.s);
out: key_put(key); pr_debug("%s() = %d\n", __func__, ret);
Hi Roberto,
On Fri, 2022-11-04 at 13:20 +0100, Roberto Sassu wrote:
From: Roberto Sassu roberto.sassu@huawei.com
Commit ac4e97abce9b8 ("scatterlist: sg_set_buf() argument must be in linear mapping") requires that both the signature and the digest resides in the linear mapping area.
However, more recently commit ba14a194a434c ("fork: Add generic vmalloced stack support"), made it possible to move the stack in the vmalloc area, which could make the requirement of the first commit not satisfied anymore.
If CONFIG_SG=y and CONFIG_VMAP_STACK=y, the following BUG() is triggered:
^CONFIG_DEBUG_SG
[ 467.077359] kernel BUG at include/linux/scatterlist.h:163! [ 467.077939] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[...]
[ 467.095225] Call Trace: [ 467.096088] <TASK> [ 467.096928] ? rcu_read_lock_held_common+0xe/0x50 [ 467.097569] ? rcu_read_lock_sched_held+0x13/0x70 [ 467.098123] ? trace_hardirqs_on+0x2c/0xd0 [ 467.098647] ? public_key_verify_signature+0x470/0x470 [ 467.099237] asymmetric_verify+0x14c/0x300 [ 467.099869] evm_verify_hmac+0x245/0x360 [ 467.100391] evm_inode_setattr+0x43/0x190
The failure happens only for the digest, as the pointer comes from the stack, and not for the signature, which instead was allocated by vfs_getxattr_alloc().
Only after enabling CONFIG_DEBUG_SG does EVM fail.
Fix this by making a copy of both in asymmetric_verify(), so that the linear mapping requirement is always satisfied, regardless of the caller.
As only EVM is affected, it would make more sense to limit the change to EVM.
On Tue, 2022-11-22 at 14:39 -0500, Mimi Zohar wrote:
Hi Roberto,
On Fri, 2022-11-04 at 13:20 +0100, Roberto Sassu wrote:
From: Roberto Sassu roberto.sassu@huawei.com
Commit ac4e97abce9b8 ("scatterlist: sg_set_buf() argument must be in linear mapping") requires that both the signature and the digest resides in the linear mapping area.
However, more recently commit ba14a194a434c ("fork: Add generic vmalloced stack support"), made it possible to move the stack in the vmalloc area, which could make the requirement of the first commit not satisfied anymore.
If CONFIG_SG=y and CONFIG_VMAP_STACK=y, the following BUG() is triggered:
^CONFIG_DEBUG_SG
[ 467.077359] kernel BUG at include/linux/scatterlist.h:163! [ 467.077939] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[...]
[ 467.095225] Call Trace: [ 467.096088] <TASK> [ 467.096928] ? rcu_read_lock_held_common+0xe/0x50 [ 467.097569] ? rcu_read_lock_sched_held+0x13/0x70 [ 467.098123] ? trace_hardirqs_on+0x2c/0xd0 [ 467.098647] ? public_key_verify_signature+0x470/0x470 [ 467.099237] asymmetric_verify+0x14c/0x300 [ 467.099869] evm_verify_hmac+0x245/0x360 [ 467.100391] evm_inode_setattr+0x43/0x190
The failure happens only for the digest, as the pointer comes from the stack, and not for the signature, which instead was allocated by vfs_getxattr_alloc().
Only after enabling CONFIG_DEBUG_SG does EVM fail.
Fix this by making a copy of both in asymmetric_verify(), so that the linear mapping requirement is always satisfied, regardless of the caller.
As only EVM is affected, it would make more sense to limit the change to EVM.
I found another occurrence:
static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, struct evm_ima_xattr_data *xattr_value, int xattr_len, enum integrity_status *status, const char **cause) {
[...]
rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, (const char *)xattr_value, xattr_len, hash.digest, hash.hdr.length);
Should I do two patches?
Thanks
Roberto
On Wed, 2022-11-23 at 13:56 +0100, Roberto Sassu wrote:
On Tue, 2022-11-22 at 14:39 -0500, Mimi Zohar wrote:
Hi Roberto,
On Fri, 2022-11-04 at 13:20 +0100, Roberto Sassu wrote:
From: Roberto Sassu roberto.sassu@huawei.com
Commit ac4e97abce9b8 ("scatterlist: sg_set_buf() argument must be in linear mapping") requires that both the signature and the digest resides in the linear mapping area.
However, more recently commit ba14a194a434c ("fork: Add generic vmalloced stack support"), made it possible to move the stack in the vmalloc area, which could make the requirement of the first commit not satisfied anymore.
If CONFIG_SG=y and CONFIG_VMAP_STACK=y, the following BUG() is triggered:
^CONFIG_DEBUG_SG
[ 467.077359] kernel BUG at include/linux/scatterlist.h:163! [ 467.077939] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[...]
[ 467.095225] Call Trace: [ 467.096088] <TASK> [ 467.096928] ? rcu_read_lock_held_common+0xe/0x50 [ 467.097569] ? rcu_read_lock_sched_held+0x13/0x70 [ 467.098123] ? trace_hardirqs_on+0x2c/0xd0 [ 467.098647] ? public_key_verify_signature+0x470/0x470 [ 467.099237] asymmetric_verify+0x14c/0x300 [ 467.099869] evm_verify_hmac+0x245/0x360 [ 467.100391] evm_inode_setattr+0x43/0x190
The failure happens only for the digest, as the pointer comes from the stack, and not for the signature, which instead was allocated by vfs_getxattr_alloc().
Only after enabling CONFIG_DEBUG_SG does EVM fail.
Fix this by making a copy of both in asymmetric_verify(), so that the linear mapping requirement is always satisfied, regardless of the caller.
As only EVM is affected, it would make more sense to limit the change to EVM.
I found another occurrence:
static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, struct evm_ima_xattr_data *xattr_value, int xattr_len, enum integrity_status *status, const char **cause) {
[...]
rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, (const char *)xattr_value, xattr_len, hash.digest, hash.hdr.length);Should I do two patches?
I'm just not getting it. Why did you enable CONFIG_DEBUG_SIG? Were you testing random kernel configs? Are you actually seeing signature verifications errors without it enabled? Or is it causing other problems? Is the "BUG_ON" still needed?
If you're going to fix the EVM and IMA callers, then make them separate patches.
On Wed, 2022-11-23 at 08:40 -0500, Mimi Zohar wrote:
On Wed, 2022-11-23 at 13:56 +0100, Roberto Sassu wrote:
On Tue, 2022-11-22 at 14:39 -0500, Mimi Zohar wrote:
Hi Roberto,
On Fri, 2022-11-04 at 13:20 +0100, Roberto Sassu wrote:
From: Roberto Sassu roberto.sassu@huawei.com
Commit ac4e97abce9b8 ("scatterlist: sg_set_buf() argument must be in linear mapping") requires that both the signature and the digest resides in the linear mapping area.
However, more recently commit ba14a194a434c ("fork: Add generic vmalloced stack support"), made it possible to move the stack in the vmalloc area, which could make the requirement of the first commit not satisfied anymore.
If CONFIG_SG=y and CONFIG_VMAP_STACK=y, the following BUG() is triggered:
^CONFIG_DEBUG_SG
[ 467.077359] kernel BUG at include/linux/scatterlist.h:163! [ 467.077939] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[...]
[ 467.095225] Call Trace: [ 467.096088] <TASK> [ 467.096928] ? rcu_read_lock_held_common+0xe/0x50 [ 467.097569] ? rcu_read_lock_sched_held+0x13/0x70 [ 467.098123] ? trace_hardirqs_on+0x2c/0xd0 [ 467.098647] ? public_key_verify_signature+0x470/0x470 [ 467.099237] asymmetric_verify+0x14c/0x300 [ 467.099869] evm_verify_hmac+0x245/0x360 [ 467.100391] evm_inode_setattr+0x43/0x190
The failure happens only for the digest, as the pointer comes from the stack, and not for the signature, which instead was allocated by vfs_getxattr_alloc().
Only after enabling CONFIG_DEBUG_SG does EVM fail.
Fix this by making a copy of both in asymmetric_verify(), so that the linear mapping requirement is always satisfied, regardless of the caller.
As only EVM is affected, it would make more sense to limit the change to EVM.
I found another occurrence:
static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, struct evm_ima_xattr_data *xattr_value, int xattr_len, enum integrity_status *status, const char **cause) {
[...]
rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, (const char *)xattr_value, xattr_len, hash.digest, hash.hdr.length);Should I do two patches?
I'm just not getting it. Why did you enable CONFIG_DEBUG_SIG? Were you testing random kernel configs? Are you actually seeing signature verifications errors without it enabled? Or is it causing other problems? Is the "BUG_ON" still needed?
When I test patches, I tend to enable more debugging options.
To be honest, I didn't check if there is any issue without enabling CONFIG_DEBUG_SG. I thought that if there is a linear mapping requirement, that should be satisfied regardless of whether the debugging option is enabled or not.
+ Rusty, Jens for explanations.
If you're going to fix the EVM and IMA callers, then make them separate patches.
Ok.
Thanks
Roberto
On Wed, 2022-11-23 at 14:49 +0100, Roberto Sassu wrote:
On Wed, 2022-11-23 at 08:40 -0500, Mimi Zohar wrote:
On Wed, 2022-11-23 at 13:56 +0100, Roberto Sassu wrote:
On Tue, 2022-11-22 at 14:39 -0500, Mimi Zohar wrote:
Hi Roberto,
On Fri, 2022-11-04 at 13:20 +0100, Roberto Sassu wrote:
From: Roberto Sassu roberto.sassu@huawei.com
Commit ac4e97abce9b8 ("scatterlist: sg_set_buf() argument must be in linear mapping") requires that both the signature and the digest resides in the linear mapping area.
However, more recently commit ba14a194a434c ("fork: Add generic vmalloced stack support"), made it possible to move the stack in the vmalloc area, which could make the requirement of the first commit not satisfied anymore.
If CONFIG_SG=y and CONFIG_VMAP_STACK=y, the following BUG() is triggered:
^CONFIG_DEBUG_SG
[ 467.077359] kernel BUG at include/linux/scatterlist.h:163! [ 467.077939] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[...]
[ 467.095225] Call Trace: [ 467.096088] <TASK> [ 467.096928] ? rcu_read_lock_held_common+0xe/0x50 [ 467.097569] ? rcu_read_lock_sched_held+0x13/0x70 [ 467.098123] ? trace_hardirqs_on+0x2c/0xd0 [ 467.098647] ? public_key_verify_signature+0x470/0x470 [ 467.099237] asymmetric_verify+0x14c/0x300 [ 467.099869] evm_verify_hmac+0x245/0x360 [ 467.100391] evm_inode_setattr+0x43/0x190
The failure happens only for the digest, as the pointer comes from the stack, and not for the signature, which instead was allocated by vfs_getxattr_alloc().
Only after enabling CONFIG_DEBUG_SG does EVM fail.
Fix this by making a copy of both in asymmetric_verify(), so that the linear mapping requirement is always satisfied, regardless of the caller.
As only EVM is affected, it would make more sense to limit the change to EVM.
I found another occurrence:
static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, struct evm_ima_xattr_data *xattr_value, int xattr_len, enum integrity_status *status, const char **cause) {
[...]
rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, (const char *)xattr_value, xattr_len, hash.digest, hash.hdr.length);Should I do two patches?
I'm just not getting it. Why did you enable CONFIG_DEBUG_SIG? Were you testing random kernel configs? Are you actually seeing signature verifications errors without it enabled? Or is it causing other problems? Is the "BUG_ON" still needed?
When I test patches, I tend to enable more debugging options.
To be honest, I didn't check if there is any issue without enabling CONFIG_DEBUG_SG. I thought that if there is a linear mapping requirement, that should be satisfied regardless of whether the debugging option is enabled or not.
- Rusty, Jens for explanations.
Trying to answer the question, with the help of an old discussion:
https://groups.google.com/g/linux.kernel/c/dpIoiY_qSGc
sg_set_buf() calls virt_to_page() to get the page to start from. But if the buffer spans in two pages, that would not work in the vmalloc area, since there is no guarantee that the next page is adjiacent.
For small areas, much smaller than the page size, it is unlikely that the situation above would happen. So, integrity_digsig_verify() will likely succeeed. Although it is possible that it fails if there are data in the next page.
Roberto
On Wed, 2022-11-30 at 15:41 +0100, Roberto Sassu wrote:
On Wed, 2022-11-23 at 14:49 +0100, Roberto Sassu wrote:
On Wed, 2022-11-23 at 08:40 -0500, Mimi Zohar wrote:
On Wed, 2022-11-23 at 13:56 +0100, Roberto Sassu wrote:
On Tue, 2022-11-22 at 14:39 -0500, Mimi Zohar wrote:
Hi Roberto,
On Fri, 2022-11-04 at 13:20 +0100, Roberto Sassu wrote:
From: Roberto Sassu roberto.sassu@huawei.com
Commit ac4e97abce9b8 ("scatterlist: sg_set_buf() argument must be in linear mapping") requires that both the signature and the digest resides in the linear mapping area.
However, more recently commit ba14a194a434c ("fork: Add generic vmalloced stack support"), made it possible to move the stack in the vmalloc area, which could make the requirement of the first commit not satisfied anymore.
If CONFIG_SG=y and CONFIG_VMAP_STACK=y, the following BUG() is triggered:
^CONFIG_DEBUG_SG
[ 467.077359] kernel BUG at include/linux/scatterlist.h:163! [ 467.077939] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[...]
[ 467.095225] Call Trace: [ 467.096088] <TASK> [ 467.096928] ? rcu_read_lock_held_common+0xe/0x50 [ 467.097569] ? rcu_read_lock_sched_held+0x13/0x70 [ 467.098123] ? trace_hardirqs_on+0x2c/0xd0 [ 467.098647] ? public_key_verify_signature+0x470/0x470 [ 467.099237] asymmetric_verify+0x14c/0x300 [ 467.099869] evm_verify_hmac+0x245/0x360 [ 467.100391] evm_inode_setattr+0x43/0x190
The failure happens only for the digest, as the pointer comes from the stack, and not for the signature, which instead was allocated by vfs_getxattr_alloc().
Only after enabling CONFIG_DEBUG_SG does EVM fail.
Fix this by making a copy of both in asymmetric_verify(), so that the linear mapping requirement is always satisfied, regardless of the caller.
As only EVM is affected, it would make more sense to limit the change to EVM.
I found another occurrence:
static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, struct evm_ima_xattr_data *xattr_value, int xattr_len, enum integrity_status *status, const char **cause) {
[...]
rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, (const char *)xattr_value, xattr_len, hash.digest, hash.hdr.length);Should I do two patches?
I'm just not getting it. Why did you enable CONFIG_DEBUG_SIG? Were you testing random kernel configs? Are you actually seeing signature verifications errors without it enabled? Or is it causing other problems? Is the "BUG_ON" still needed?
When I test patches, I tend to enable more debugging options.
To be honest, I didn't check if there is any issue without enabling CONFIG_DEBUG_SG. I thought that if there is a linear mapping requirement, that should be satisfied regardless of whether the debugging option is enabled or not.
- Rusty, Jens for explanations.
Trying to answer the question, with the help of an old discussion:
https://groups.google.com/g/linux.kernel/c/dpIoiY_qSGc
sg_set_buf() calls virt_to_page() to get the page to start from. But if the buffer spans in two pages, that would not work in the vmalloc area, since there is no guarantee that the next page is adjiacent.
For small areas, much smaller than the page size, it is unlikely that the situation above would happen. So, integrity_digsig_verify() will likely succeeed. Although it is possible that it fails if there are data in the next page.
Thanks, Roberto. Confirmed that as the patch description indicates, without CONFIG_VMAP_STACK configured and with CONFIG_DEBUG_SG enabled there isn't a bug. Does it make sense to limit this change to just CONFIG_VMAP_STACK?
On Wed, 2022-11-30 at 11:22 -0500, Mimi Zohar wrote:
On Wed, 2022-11-30 at 15:41 +0100, Roberto Sassu wrote:
On Wed, 2022-11-23 at 14:49 +0100, Roberto Sassu wrote:
On Wed, 2022-11-23 at 08:40 -0500, Mimi Zohar wrote:
On Wed, 2022-11-23 at 13:56 +0100, Roberto Sassu wrote:
On Tue, 2022-11-22 at 14:39 -0500, Mimi Zohar wrote:
Hi Roberto,
On Fri, 2022-11-04 at 13:20 +0100, Roberto Sassu wrote: > From: Roberto Sassu roberto.sassu@huawei.com > > Commit ac4e97abce9b8 ("scatterlist: sg_set_buf() argument must be in linear > mapping") requires that both the signature and the digest resides in the > linear mapping area. > > However, more recently commit ba14a194a434c ("fork: Add generic vmalloced > stack support"), made it possible to move the stack in the vmalloc area, > which could make the requirement of the first commit not satisfied anymore. > > If CONFIG_SG=y and CONFIG_VMAP_STACK=y, the following BUG() is triggered:
^CONFIG_DEBUG_SG
> [ 467.077359] kernel BUG at include/linux/scatterlist.h:163! > [ 467.077939] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI > > [...] > > [ 467.095225] Call Trace: > [ 467.096088] <TASK> > [ 467.096928] ? rcu_read_lock_held_common+0xe/0x50 > [ 467.097569] ? rcu_read_lock_sched_held+0x13/0x70 > [ 467.098123] ? trace_hardirqs_on+0x2c/0xd0 > [ 467.098647] ? public_key_verify_signature+0x470/0x470 > [ 467.099237] asymmetric_verify+0x14c/0x300 > [ 467.099869] evm_verify_hmac+0x245/0x360 > [ 467.100391] evm_inode_setattr+0x43/0x190 > > The failure happens only for the digest, as the pointer comes from the > stack, and not for the signature, which instead was allocated by > vfs_getxattr_alloc().
Only after enabling CONFIG_DEBUG_SG does EVM fail.
> Fix this by making a copy of both in asymmetric_verify(), so that the > linear mapping requirement is always satisfied, regardless of the caller.
As only EVM is affected, it would make more sense to limit the change to EVM.
I found another occurrence:
static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, struct evm_ima_xattr_data *xattr_value, int xattr_len, enum integrity_status *status, const char **cause) {
[...]
rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, (const char *)xattr_value, xattr_len, hash.digest, hash.hdr.length);Should I do two patches?
I'm just not getting it. Why did you enable CONFIG_DEBUG_SIG? Were you testing random kernel configs? Are you actually seeing signature verifications errors without it enabled? Or is it causing other problems? Is the "BUG_ON" still needed?
When I test patches, I tend to enable more debugging options.
To be honest, I didn't check if there is any issue without enabling CONFIG_DEBUG_SG. I thought that if there is a linear mapping requirement, that should be satisfied regardless of whether the debugging option is enabled or not.
- Rusty, Jens for explanations.
Trying to answer the question, with the help of an old discussion:
https://groups.google.com/g/linux.kernel/c/dpIoiY_qSGc
sg_set_buf() calls virt_to_page() to get the page to start from. But if the buffer spans in two pages, that would not work in the vmalloc area, since there is no guarantee that the next page is adjiacent.
For small areas, much smaller than the page size, it is unlikely that the situation above would happen. So, integrity_digsig_verify() will likely succeeed. Although it is possible that it fails if there are data in the next page.
Thanks, Roberto. Confirmed that as the patch description indicates, without CONFIG_VMAP_STACK configured and with CONFIG_DEBUG_SG enabled there isn't a bug. Does it make sense to limit this change to just CONFIG_VMAP_STACK?
Yes, I agree.
Roberto
linux-stable-mirror@lists.linaro.org
-
Mimi Zohar -
Roberto Sassu