| Message ID | 20250112072925.1774-1-tanyaagarwal25699@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [V2] security: fix typos and spelling errors | expand |
On Sun, Jan 12, 2025 at 2:30 AM Tanya Agarwal <tanyaagarwal25699@gmail.com> wrote: > > From: Tanya Agarwal <tanyaagarwal25699@gmail.com> > > Fix typos and spelling errors in security module comments that were > identified using the codespell tool. > No functional changes - documentation only. > > Signed-off-by: Tanya Agarwal <tanyaagarwal25699@gmail.com> > --- > Thanks Günther, for catching this error. > The irony of having a spelling mistake in a patch that fixes spelling > mistakes is not lost on me :) > > I've fixed it in V2 of the patch. Thank you for the careful review! > > V2: fix spelling mistake - s/beeen/been/ > > security/apparmor/apparmorfs.c | 6 +++--- > security/apparmor/domain.c | 4 ++-- > security/apparmor/label.c | 2 +- > security/apparmor/lsm.c | 2 +- > security/apparmor/policy.c | 4 ++-- > security/integrity/evm/evm_crypto.c | 2 +- > security/integrity/evm/evm_main.c | 2 +- > security/integrity/ima/ima_main.c | 6 +++--- > security/landlock/ruleset.c | 2 +- > security/selinux/avc.c | 2 +- > security/smack/smack.h | 2 +- > security/smack/smack_access.c | 4 ++-- > security/smack/smack_lsm.c | 6 +++--- > security/smack/smackfs.c | 2 +- > security/tomoyo/domain.c | 2 +- > 15 files changed, 24 insertions(+), 24 deletions(-) Hi Tanya, Ideally this patchset would be split into into seperate, independent patches, one for AppArmor, one for IMA/EVM, one for Landlock, one for SELinux, one for Smack, and one for TOMOYO. This allows for each LSM maintainer to review and merge these fixes individually as opposed to requiring every LSM maintainer to ACK this patch before it can be merged. There is also a risk, as with any cross-subsystem patch, that this patch will cause merge conflicts in the future as there is the possibility of multiple development trees touching the same file region, function, etc. As a general rule, if you have a single patch that touches multiple kernel subsystems, and the changes in each subsystem can be applied independently, you really should split the patch into subsystem specific patches. You also should post these patches independently, and not as a single patchset, as a single patchset crossing multiple subsystems can sometimes cause some confusion among maintainers about who is going to be responsible for handling the patchset (even if all the patches are split properly). -- paul-moore.com
On 2025/01/13 1:36, Paul Moore wrote: > Hi Tanya, > > Ideally this patchset would be split into into seperate, independent > patches, one for AppArmor, one for IMA/EVM, one for Landlock, one for > SELinux, one for Smack, and one for TOMOYO. I don't think we need to split this patchset into individual modules, especially because this patchset does not affect the result of kernel build. We sometimes need to do "git bisect", and reducing number of commits helps saving building time and testing time for bisection.
On Sun, Jan 12, 2025 at 7:00 PM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > On 2025/01/13 1:36, Paul Moore wrote: > > Hi Tanya, > > > > Ideally this patchset would be split into into seperate, independent > > patches, one for AppArmor, one for IMA/EVM, one for Landlock, one for > > SELinux, one for Smack, and one for TOMOYO. > > I don't think we need to split this patchset into individual modules, > especially because this patchset does not affect the result of kernel build. > We sometimes need to do "git bisect", and reducing number of commits helps > saving building time and testing time for bisection. Merge conflicts and spending time having to coordinate maintainer ACKs is a real time cost. Split the patch please.
On Sun, Jan 12, 2025 at 12:59:27PM +0530, Tanya Agarwal wrote: > From: Tanya Agarwal <tanyaagarwal25699@gmail.com> > > Fix typos and spelling errors in security module comments that were > identified using the codespell tool. > No functional changes - documentation only. > > Signed-off-by: Tanya Agarwal <tanyaagarwal25699@gmail.com> > --- > Thanks Günther, for catching this error. > The irony of having a spelling mistake in a patch that fixes spelling > mistakes is not lost on me :) > > I've fixed it in V2 of the patch. Thank you for the careful review! > > V2: fix spelling mistake - s/beeen/been/ > > security/apparmor/apparmorfs.c | 6 +++--- > security/apparmor/domain.c | 4 ++-- > security/apparmor/label.c | 2 +- > security/apparmor/lsm.c | 2 +- > security/apparmor/policy.c | 4 ++-- > security/integrity/evm/evm_crypto.c | 2 +- > security/integrity/evm/evm_main.c | 2 +- > security/integrity/ima/ima_main.c | 6 +++--- > security/landlock/ruleset.c | 2 +- > security/selinux/avc.c | 2 +- > security/smack/smack.h | 2 +- > security/smack/smack_access.c | 4 ++-- > security/smack/smack_lsm.c | 6 +++--- > security/smack/smackfs.c | 2 +- > security/tomoyo/domain.c | 2 +- > 15 files changed, 24 insertions(+), 24 deletions(-) > [...] > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > index 9b87556b03a7..cdb8c7419d7e 100644 > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -983,9 +983,9 @@ int process_buffer_measurement(struct mnt_idmap *idmap, > } > > /* > - * Both LSM hooks and auxilary based buffer measurements are > - * based on policy. To avoid code duplication, differentiate > - * between the LSM hooks and auxilary buffer measurements, > + * Both LSM hooks and auxiliary based buffer measurements are > + * based on policy. To avoid code duplication, differentiate ^^^ (Small nit:) This change from two-spaces-after-the-dot to a single space looks like it happened accidentally. The two-space style is dominant in the ima_main.c file. (However, I am not involved in IMA and others have more authority to review this part. As Paul also said, reviews tend to go smoother when the scope for the patch is a single subsystem - it makes the responsibilities clearer.) > + * between the LSM hooks and auxiliary buffer measurements, > * retrieving the policy rule information only for the LSM hook > * buffer measurements. > */ –Günther
On Mon, 2025-01-13 at 15:50 +0100, Günther Noack wrote: > On Sun, Jan 12, 2025 at 12:59:27PM +0530, Tanya Agarwal wrote: > > From: Tanya Agarwal <tanyaagarwal25699@gmail.com> > > > > Fix typos and spelling errors in security module comments that were > > identified using the codespell tool. > > No functional changes - documentation only. > > > > Signed-off-by: Tanya Agarwal <tanyaagarwal25699@gmail.com> > > --- > > Thanks Günther, for catching this error. > > The irony of having a spelling mistake in a patch that fixes spelling > > mistakes is not lost on me :) > > > > I've fixed it in V2 of the patch. Thank you for the careful review! > > > > V2: fix spelling mistake - s/beeen/been/ > > > > security/apparmor/apparmorfs.c | 6 +++--- > > security/apparmor/domain.c | 4 ++-- > > security/apparmor/label.c | 2 +- > > security/apparmor/lsm.c | 2 +- > > security/apparmor/policy.c | 4 ++-- > > security/integrity/evm/evm_crypto.c | 2 +- > > security/integrity/evm/evm_main.c | 2 +- > > security/integrity/ima/ima_main.c | 6 +++--- > > security/landlock/ruleset.c | 2 +- > > security/selinux/avc.c | 2 +- > > security/smack/smack.h | 2 +- > > security/smack/smack_access.c | 4 ++-- > > security/smack/smack_lsm.c | 6 +++--- > > security/smack/smackfs.c | 2 +- > > security/tomoyo/domain.c | 2 +- > > 15 files changed, 24 insertions(+), 24 deletions(-) > > > > [...] > > > diff --git a/security/integrity/ima/ima_main.c > > b/security/integrity/ima/ima_main.c > > index 9b87556b03a7..cdb8c7419d7e 100644 > > --- a/security/integrity/ima/ima_main.c > > +++ b/security/integrity/ima/ima_main.c > > @@ -983,9 +983,9 @@ int process_buffer_measurement(struct mnt_idmap *idmap, > > } > > > > /* > > - * Both LSM hooks and auxilary based buffer measurements are > > - * based on policy. To avoid code duplication, differentiate > > - * between the LSM hooks and auxilary buffer measurements, > > + * Both LSM hooks and auxiliary based buffer measurements are > > + * based on policy. To avoid code duplication, differentiate > ^^^ > > (Small nit:) This change from two-spaces-after-the-dot to a single > space looks like it happened accidentally. The two-space style is > dominant in the ima_main.c file. > > (However, I am not involved in IMA and others have more authority to > review this part. As Paul also said, reviews tend to go smoother when > the scope for the patch is a single subsystem - it makes the > responsibilities clearer.) > > > + * between the LSM hooks and auxiliary buffer measurements, The spelling mistake being corrected in two places is auxilary -> auxiliary. Thank you for noticing the unnecessary change from two spaces to one space as well. FYI, I agree with Paul the patch should be split. Reviewed-by: Mimi Zohar <zohar@linux.ibm.com> > > * retrieving the policy rule information only for the LSM hook > > * buffer measurements. > > */ > > –Günther
On Tue, Jan 14, 2025 at 12:49 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > On Mon, 2025-01-13 at 15:50 +0100, Günther Noack wrote: > > On Sun, Jan 12, 2025 at 12:59:27PM +0530, Tanya Agarwal wrote: > > > From: Tanya Agarwal <tanyaagarwal25699@gmail.com> > > > > > > Fix typos and spelling errors in security module comments that were > > > identified using the codespell tool. > > > No functional changes - documentation only. > > > > > > Signed-off-by: Tanya Agarwal <tanyaagarwal25699@gmail.com> > > > --- > > > Thanks Günther, for catching this error. > > > The irony of having a spelling mistake in a patch that fixes spelling > > > mistakes is not lost on me :) > > > > > > I've fixed it in V2 of the patch. Thank you for the careful review! > > > > > > V2: fix spelling mistake - s/beeen/been/ > > > > > > security/apparmor/apparmorfs.c | 6 +++--- > > > security/apparmor/domain.c | 4 ++-- > > > security/apparmor/label.c | 2 +- > > > security/apparmor/lsm.c | 2 +- > > > security/apparmor/policy.c | 4 ++-- > > > security/integrity/evm/evm_crypto.c | 2 +- > > > security/integrity/evm/evm_main.c | 2 +- > > > security/integrity/ima/ima_main.c | 6 +++--- > > > security/landlock/ruleset.c | 2 +- > > > security/selinux/avc.c | 2 +- > > > security/smack/smack.h | 2 +- > > > security/smack/smack_access.c | 4 ++-- > > > security/smack/smack_lsm.c | 6 +++--- > > > security/smack/smackfs.c | 2 +- > > > security/tomoyo/domain.c | 2 +- > > > 15 files changed, 24 insertions(+), 24 deletions(-) > > > > > > > [...] > > > > > diff --git a/security/integrity/ima/ima_main.c > > > b/security/integrity/ima/ima_main.c > > > index 9b87556b03a7..cdb8c7419d7e 100644 > > > --- a/security/integrity/ima/ima_main.c > > > +++ b/security/integrity/ima/ima_main.c > > > @@ -983,9 +983,9 @@ int process_buffer_measurement(struct mnt_idmap *idmap, > > > } > > > > > > /* > > > - * Both LSM hooks and auxilary based buffer measurements are > > > - * based on policy. To avoid code duplication, differentiate > > > - * between the LSM hooks and auxilary buffer measurements, > > > + * Both LSM hooks and auxiliary based buffer measurements are > > > + * based on policy. To avoid code duplication, differentiate > > ^^^ > > > > (Small nit:) This change from two-spaces-after-the-dot to a single > > space looks like it happened accidentally. The two-space style is > > dominant in the ima_main.c file. > > > > (However, I am not involved in IMA and others have more authority to > > review this part. As Paul also said, reviews tend to go smoother when > > the scope for the patch is a single subsystem - it makes the > > responsibilities clearer.) > > > > > + * between the LSM hooks and auxiliary buffer measurements, > > The spelling mistake being corrected in two places is auxilary -> auxiliary. Thank > you for noticing the unnecessary change from two spaces to one space as well. > > FYI, I agree with Paul the patch should be split. > > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com> > Hi All, Thanks for the review. Sure, I'll split patches of different security subsystems so, that they are easy for maintainers to merge. Regards, Tanya
On Tue, Jan 14, 2025 at 11:13 AM Tanya Agarwal <tanyaagarwal25699@gmail.com> wrote: > > Hi All, > Thanks for the review. > Sure, I'll split patches of different security subsystems so, that > they are easy for maintainers to merge. Thanks!
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 2c0185ebc900..0c2f248d31bf 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -43,7 +43,7 @@ * The interface is split into two main components based on their function * a securityfs component: * used for static files that are always available, and which allows - * userspace to specificy the location of the security filesystem. + * userspace to specify the location of the security filesystem. * * fns and data are prefixed with * aa_sfs_ @@ -204,7 +204,7 @@ static struct file_system_type aafs_ops = { /** * __aafs_setup_d_inode - basic inode setup for apparmorfs * @dir: parent directory for the dentry - * @dentry: dentry we are seting the inode up for + * @dentry: dentry we are setting the inode up for * @mode: permissions the file should have * @data: data to store on inode.i_private, available in open() * @link: if symlink, symlink target string @@ -2244,7 +2244,7 @@ static void *p_next(struct seq_file *f, void *p, loff_t *pos) /** * p_stop - stop depth first traversal * @f: seq_file we are filling - * @p: the last profile writen + * @p: the last profile written * * Release all locking done by p_start/p_next on namespace tree */ diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 5939bd9a9b9b..d959931eac28 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -755,7 +755,7 @@ static int profile_onexec(const struct cred *subj_cred, /* change_profile on exec already granted */ /* * NOTE: Domain transitions from unconfined are allowed - * even when no_new_privs is set because this aways results + * even when no_new_privs is set because this always results * in a further reduction of permissions. */ return 0; @@ -926,7 +926,7 @@ int apparmor_bprm_creds_for_exec(struct linux_binprm *bprm) * * NOTE: Domain transitions from unconfined and to stacked * subsets are allowed even when no_new_privs is set because this - * aways results in a further reduction of permissions. + * always results in a further reduction of permissions. */ if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) && !unconfined(label) && diff --git a/security/apparmor/label.c b/security/apparmor/label.c index 91483ecacc16..8bcff51becb8 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -1456,7 +1456,7 @@ bool aa_update_label_name(struct aa_ns *ns, struct aa_label *label, gfp_t gfp) /* * cached label name is present and visible - * @label->hname only exists if label is namespace hierachical + * @label->hname only exists if label is namespace hierarchical */ static inline bool use_label_hname(struct aa_ns *ns, struct aa_label *label, int flags) diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 1edc12862a7d..04bf5d2f6e00 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -2006,7 +2006,7 @@ static int __init alloc_buffers(void) * two should be enough, with more CPUs it is possible that more * buffers will be used simultaneously. The preallocated pool may grow. * This preallocation has also the side-effect that AppArmor will be - * disabled early at boot if aa_g_path_max is extremly high. + * disabled early at boot if aa_g_path_max is extremely high. */ if (num_online_cpus() > 1) num = 4 + RESERVE_COUNT; diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index d0244fab0653..5cec3efc4794 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -463,7 +463,7 @@ static struct aa_policy *__lookup_parent(struct aa_ns *ns, } /** - * __create_missing_ancestors - create place holders for missing ancestores + * __create_missing_ancestors - create place holders for missing ancestors * @ns: namespace to lookup profile in (NOT NULL) * @hname: hierarchical profile name to find parent of (NOT NULL) * @gfp: type of allocation. @@ -1068,7 +1068,7 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label, goto out; /* ensure that profiles are all for the same ns - * TODO: update locking to remove this constaint. All profiles in + * TODO: update locking to remove this constraint. All profiles in * the load set must succeed as a set or the load will * fail. Sort ent list and take ns locks in hierarchy order */ diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 7c06ffd633d2..a5e730ffda57 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -180,7 +180,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, } /* - * Dump large security xattr values as a continuous ascii hexademical string. + * Dump large security xattr values as a continuous ascii hexadecimal string. * (pr_debug is limited to 64 bytes.) */ static void dump_security_xattr_l(const char *prefix, const void *src, diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 377e57e9084f..0add782e73ba 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -169,7 +169,7 @@ static int is_unsupported_hmac_fs(struct dentry *dentry) * and compare it against the stored security.evm xattr. * * For performance: - * - use the previoulsy retrieved xattr value and length to calculate the + * - use the previously retrieved xattr value and length to calculate the * HMAC.) * - cache the verification result in the iint, when available. * diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 9b87556b03a7..cdb8c7419d7e 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -983,9 +983,9 @@ int process_buffer_measurement(struct mnt_idmap *idmap, } /* - * Both LSM hooks and auxilary based buffer measurements are - * based on policy. To avoid code duplication, differentiate - * between the LSM hooks and auxilary buffer measurements, + * Both LSM hooks and auxiliary based buffer measurements are + * based on policy. To avoid code duplication, differentiate + * between the LSM hooks and auxiliary buffer measurements, * retrieving the policy rule information only for the LSM hook * buffer measurements. */ diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c index a93bdbf52fff..c464d1f84792 100644 --- a/security/landlock/ruleset.c +++ b/security/landlock/ruleset.c @@ -121,7 +121,7 @@ create_rule(const struct landlock_id id, return ERR_PTR(-ENOMEM); RB_CLEAR_NODE(&new_rule->node); if (is_object_pointer(id.type)) { - /* This should be catched by insert_rule(). */ + /* This should have been caught by insert_rule(). */ WARN_ON_ONCE(!id.key.object); landlock_get_object(id.key.object); } diff --git a/security/selinux/avc.c b/security/selinux/avc.c index 1f2680bcc43a..4b4837a20225 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -936,7 +936,7 @@ static void avc_flush(void) spin_lock_irqsave(lock, flag); /* - * With preemptable RCU, the outer spinlock does not + * With preemptible RCU, the outer spinlock does not * prevent RCU grace periods from ending. */ rcu_read_lock(); diff --git a/security/smack/smack.h b/security/smack/smack.h index dbf8d7226eb5..ca38e145f364 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -42,7 +42,7 @@ /* * This is the repository for labels seen so that it is - * not necessary to keep allocating tiny chuncks of memory + * not necessary to keep allocating tiny chunks of memory * and so that they can be shared. * * Labels are never modified in place. Anytime a label diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 585e5e35710b..5c17aee5dd78 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -242,7 +242,7 @@ int smk_tskacc(struct task_smack *tsp, struct smack_known *obj_known, } /* - * Allow for priviliged to override policy. + * Allow for privileged to override policy. */ if (rc != 0 && smack_privileged(CAP_MAC_OVERRIDE)) rc = 0; @@ -277,7 +277,7 @@ int smk_curacc(struct smack_known *obj_known, #ifdef CONFIG_AUDIT /** - * smack_str_from_perm : helper to transalate an int to a + * smack_str_from_perm : helper to translate an int to a * readable string * @string : the string to fill * @access : the int diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 0c476282e279..85ec288eefe7 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1950,7 +1950,7 @@ static int smack_file_send_sigiotask(struct task_struct *tsk, */ file = fown->file; - /* we don't log here as rc can be overriden */ + /* we don't log here as rc can be overridden */ blob = smack_file(file); skp = *blob; rc = smk_access(skp, tkp, MAY_DELIVER, NULL); @@ -4211,7 +4211,7 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) /* * Receiving a packet requires that the other end * be able to write here. Read access is not required. - * This is the simplist possible security model + * This is the simplest possible security model * for networking. */ rc = smk_access(skp, ssp->smk_in, MAY_WRITE, &ad); @@ -4717,7 +4717,7 @@ static int smack_post_notification(const struct cred *w_cred, * @gfp: type of the memory for the allocation * * Prepare to audit cases where (@field @op @rulestr) is true. - * The label to be audited is created if necessay. + * The label to be audited is created if necessary. */ static int smack_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule, gfp_t gfp) diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index 1401412fd794..432e2d094e35 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -165,7 +165,7 @@ static int smk_cipso_doi_value = SMACK_CIPSO_DOI_DEFAULT; #define SMK_LOADLEN (SMK_LABELLEN + SMK_LABELLEN + SMK_ACCESSLEN) /* - * Stricly for CIPSO level manipulation. + * Strictly for CIPSO level manipulation. * Set the category bit number in a smack label sized buffer. */ static inline void smack_catset_bit(unsigned int cat, char *catsetp) diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c index aed9e3ef2c9e..9a1928be707d 100644 --- a/security/tomoyo/domain.c +++ b/security/tomoyo/domain.c @@ -913,7 +913,7 @@ bool tomoyo_dump_page(struct linux_binprm *bprm, unsigned long pos, #ifdef CONFIG_MMU /* * This is called at execve() time in order to dig around - * in the argv/environment of the new proceess + * in the argv/environment of the new process * (represented by bprm). */ mmap_read_lock(bprm->mm);