| Message ID | 20230919015608.8390-1-kunyu@nfschina.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v2] apparmor/file: Removing unnecessary initial values for variable pointers | expand |
On 9/18/23 18:56, Li kunyu wrote: > These variable pointers are assigned during use and do not need to be > initialized for assignment. > > Signed-off-by: Li kunyu <kunyu@nfschina.com> > --- > v2: Fix timestamp issues > > security/apparmor/file.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/security/apparmor/file.c b/security/apparmor/file.c > index 698b124e649f..12eafdf18fc0 100644 > --- a/security/apparmor/file.c > +++ b/security/apparmor/file.c > @@ -264,7 +264,7 @@ int aa_path_perm(const char *op, struct aa_label *label, > { > struct aa_perms perms = {}; > struct aa_profile *profile; > - char *buffer = NULL; > + char *buffer; this is okay > int error; > > flags |= PATH_DELEGATE_DELETED | (S_ISDIR(cond->mode) ? PATH_IS_DIR : > @@ -412,7 +412,7 @@ int aa_path_link(struct aa_label *label, struct dentry *old_dentry, > d_backing_inode(old_dentry)->i_uid, > d_backing_inode(old_dentry)->i_mode > }; > - char *buffer = NULL, *buffer2 = NULL; > + char *buffer, *buffer2; this can cause an oops if buffer2 allocation fails. There are a couple of ways I can see to fix this, do you want to take a crack at it. > struct aa_profile *profile; > int error; >
On Thu, Sep 28, 2023 at 10:36:16AM -0700, John Johansen wrote: > On 9/18/23 18:56, Li kunyu wrote: > > These variable pointers are assigned during use and do not need to be > > initialized for assignment. > > > > Signed-off-by: Li kunyu <kunyu@nfschina.com> > > --- > > v2: Fix timestamp issues > > > > security/apparmor/file.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/security/apparmor/file.c b/security/apparmor/file.c > > index 698b124e649f..12eafdf18fc0 100644 > > --- a/security/apparmor/file.c > > +++ b/security/apparmor/file.c > > @@ -264,7 +264,7 @@ int aa_path_perm(const char *op, struct aa_label *label, > > { > > struct aa_perms perms = {}; > > struct aa_profile *profile; > > - char *buffer = NULL; > > + char *buffer; > > this is okay > > > int error; > > flags |= PATH_DELEGATE_DELETED | (S_ISDIR(cond->mode) ? PATH_IS_DIR : > > @@ -412,7 +412,7 @@ int aa_path_link(struct aa_label *label, struct dentry *old_dentry, > > d_backing_inode(old_dentry)->i_uid, > > d_backing_inode(old_dentry)->i_mode > > }; > > - char *buffer = NULL, *buffer2 = NULL; > > + char *buffer, *buffer2; > > this can cause an oops if buffer2 allocation fails. There are a couple of ways I can > see to fix this, do you want to take a crack at it. > > > > struct aa_profile *profile; > > int error; I don't whether this kind of thing has become in vogue, but while indeed the first case is okay right now, it becomes more likely that a future patch to this function will inadvertently goto aa_put_buffer(buffer) before the aa_get_buffer call. I would not have NACKed an original version of this fn without the = NULL, but I'm not in favor of dropping it. -serge
diff --git a/security/apparmor/file.c b/security/apparmor/file.c index 698b124e649f..12eafdf18fc0 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -264,7 +264,7 @@ int aa_path_perm(const char *op, struct aa_label *label, { struct aa_perms perms = {}; struct aa_profile *profile; - char *buffer = NULL; + char *buffer; int error; flags |= PATH_DELEGATE_DELETED | (S_ISDIR(cond->mode) ? PATH_IS_DIR : @@ -412,7 +412,7 @@ int aa_path_link(struct aa_label *label, struct dentry *old_dentry, d_backing_inode(old_dentry)->i_uid, d_backing_inode(old_dentry)->i_mode }; - char *buffer = NULL, *buffer2 = NULL; + char *buffer, *buffer2; struct aa_profile *profile; int error;
These variable pointers are assigned during use and do not need to be initialized for assignment. Signed-off-by: Li kunyu <kunyu@nfschina.com> --- v2: Fix timestamp issues security/apparmor/file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)