On 10/15/2021 12:50 PM, Randy Dunlap wrote:
On 10/15/21 12:25 PM, Deven Bowers wrote:On 10/13/2021 3:54 PM, Randy Dunlap wrote:Hi, On 10/13/21 12:06 PM, deven.desai@xxxxxxxxxxxxxxxxxxx wrote:diff --git a/security/ipe/Kconfig b/security/ipe/Kconfig index c4503083e92d..ef556b66e674 100644 --- a/security/ipe/Kconfig +++ b/security/ipe/Kconfig @@ -17,3 +17,55 @@ menuconfig SECURITY_IPE requirements on the fly. If unsure, answer N. + +if SECURITY_IPE + +choice + prompt "Hash algorithm used in auditing policies" + default IPE_AUDIT_HASH_SHA1 + depends on AUDIT + help + Specify the hash algorithm used when auditing policies. + The hash is used to uniquely identify a policy from other + policies on the system. + + If unsure, leave default. + + config IPE_AUDIT_HASH_SHA1 + bool "sha1" + depends on CRYPTO_SHA1 + help + Use the SHA128 algorithm to hash policies + in the audit records. + + config IPE_AUDIT_HASH_SHA256 + bool "sha256" + depends on CRYPTO_SHA256 + help + Use the SHA256 algorithm to hash policies + in the audit records. + + config IPE_AUDIT_HASH_SHA384 + bool "sha384" + depends on CRYPTO_SHA512 + help + Use the SHA384 algorithm to hash policies + in the audit records + + config IPE_AUDIT_HASH_SHA512 + bool "sha512" + depends on CRYPTO_SHA512 + help + Use the SHA512 algorithm to hash policies + in the audit records +endchoice + +config IPE_AUDIT_HASH_ALG + string + depends on AUDIT + default "sha1" if IPE_AUDIT_HASH_SHA1 + default "sha256" if IPE_AUDIT_HASH_SHA256 + default "sha384" if IPE_AUDIT_HASH_SHA384 + default "sha512" if IPE_AUDIT_HASH_SHA512 + +endifPlease follow coding-style for Kconfig files: (from Documentation/process/coding-style.rst, section 10): For all of the Kconfig* configuration files throughout the source tree,the indentation is somewhat different. Lines under a ``config`` definition are indented with one tab, while help text is indented an additional twospaces.Oof. That's embarrassing. Sorry, I'll fix this for v8. While I'm at it, is the help text required for choice configs? checkpatch --strict complains with a warning without them, but I see other places in the tree where help text is omitted for these configs attached to a choice.Does checkpatch complain about what you have above or did you add that help text to keep it from complaining?
I added the help text to keep it from complaining (and added it incorrectly, clearly).
Documentation/process/* doesn't seem to have any guidance, nor Documentation/kbuild/* on whether it is safe to ignore that checkpatch warning.Yeah, I don't think that we have any good guidance on that. I would say that if the choice prompt provides good/adequate help info, then each 'config' inside the choice block does not need help text. OTOH, if the choice prompt has little/no help info, then each 'config' under it should have some useful info. I only looked in arch/x86/Kconfig, init/Kconfig, and lib/Kconfig.debug, but you can see either help text method being used in those. And then if the help text is adequate in either one of those methods, I would just ignore the checkpatch complaints. It's just a guidance tool.
Alright. I think the choice guidance is pretty clear: Specify the hash algorithm used when auditing policies. The hash is used to uniquely identify a policy from other policies on the system. So I'll remove the help text for these choices. At worst, I can make the choice text more clear.
HTH.