security: ima_file_mmap() don't just return zero
Subsystems (4)
PATCH 1/1 security: ima_file_mmap() don't just return zero
It seems like we should return an error here. That's what the comment says we should do.
I also removed an out of date comment. It wasn't needed and seemed likely to get out of date again.
---
This was found with a static checker and I have only compile tested it. The callers all seem to use the return code, but please review carefully. The code has been like this since the module was merged.
security/integrity/ima/ima_main.c +2 -3
@@ -260,18 +260,17 @@ out:* policy decision. * * Return 0 on success, an error code on failure. -* (Based on the results of appraise_measurement().) */ int ima_file_mmap (struct file *file,unsigned long prot) { -int rc; +int rc =0 ;if (!file)return 0 ;if (prot &PROT_EXEC ) rc =process_measurement (file, file->f_dentry->d_name.name,MAY_EXEC ,FILE_MMAP ); -return 0 ; +return rc; }/**
Quoted 38 lines · Dan Carpenter
It seems like we should return an error here. That's what the comment says we should do.
I also removed an out of date comment. It wasn't needed and seemed likely to get out of date again.
Signed-off-by Dan Carpenter <[email protected]>---
This was found with a static checker and I have only compile tested it. The callers all seem to use the return code, but please review carefully. The code has been like this since the module was merged.security/integrity/ima/ima_main.c +2
-4@@ -260,18 +260,17 @@ out:* policy decision. * * Return 0 on success, an error code on failure. -* (Based on the results of appraise_measurement().) */ int ima_file_mmap(struct file *file, unsigned long prot) { - int rc; + int rc = 0; if (!file) return 0; if (prot & PROT_EXEC) rc = process_measurement(file, file->f_dentry->d_name.name, MAY_EXEC, FILE_MMAP); - return 0; + return rc; } /** --
Quoted 4 lines
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Looks good, and nice work here :)
Signature · 7 lines
Regards, Vikram Dhillon ~~~ There are lots of Linux users who don't care how the kernel works, but only want to use it. That is a tribute to how good Linux is. -- Linus Torvalds
Quoted 10 lines · Mimi Zohar
Vikram Dhillon <[email protected]> wrote on 03/06/2010 04:30:33 PM:
Quoted 8 lines · Dan Carpenter
Quoted 7 lines
It seems like we should return an error here. That's what the comment says we should do.
I also removed an out of date comment. It wasn't needed and seemed likely
to get out of date again.
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Signed-off-by Dan Carpenter <[email protected]>---
The current code and comment are correct, as the return code will reflect the file's integrity appraisal, which hasn't yet been implemented.
Mimi
Oh right :) I guess we aren't at a point yet to enforce integrity, so what should the return code return? Based on the comment the code seemed okay but I think we may have to base it on measurement appraisal in LIM (then again, not too sure if this is the right approach).
Signature · 7 lines
Regards, Vikram Dhillon ~~~ There are lots of Linux users who don't care how the kernel works, but only want to use it. That is a tribute to how good Linux is. -- Linus Torvalds