On Tue, Jan 31, 2023 at 11:49:44AM +0100, Roberto Sassu wrote:
> On Mon, 2023-01-30 at 14:57 -0800, Fan Wu wrote:
> > From: Deven Bowers <deven.desai@xxxxxxxxxxxxxxxxxxx>
> > +
> > +/**
> > + * new_policy - Write handler for the securityfs node, "ipe/new_policy".
> > + * @f: Supplies a file structure representing the securityfs node.
> > + * @data: Suppleis a buffer passed to the write syscall.
>
> Typo: Suppleis.
>
Thanks for spotting the typos!
> > + * @len: Supplies the length of @data.
> > + * @offset: unused.
> > + *
> > + * Return:
> > + * * >0 - Success, Length of buffer written
> > + * * <0 - Error
> > + */
> > +static ssize_t new_policy(struct file *f, const char __user *data,
> > + size_t len, loff_t *offset)
> > +{
> > + int rc = 0;
> > + char *copy = NULL;
> > + struct ipe_policy *p = NULL;
> > +
> > + if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
> > + return -EPERM;
> > +
> > + copy = memdup_user_nul(data, len);
> > + if (IS_ERR(copy)) {
> > + rc = PTR_ERR(copy);
> > + goto err;
> > + }
> > +
> > + p = ipe_new_policy(NULL, 0, copy, len);
> > + if (IS_ERR(p)) {
> > + rc = PTR_ERR(p);
> > + goto err;
> > + }
> > +
> > + rc = ipe_new_policyfs_node(p);
> > + if (rc)
> > + goto err;
>
> Uhm, don't you need to do cleanup of allocated memory or revert the
> actions of ipe_new_policy()?
>
Yes that should be cleaned up but should be done in ipe_new_policy instead,
will add a ipe_free_policy call at the end. Thanks for pointing that out.
>
> I would like more to see all the functions managing the policy
> together. If the patch is too long, you could further split by adding
> the helpers (that don't directly deal with the policy) in a separate
> patch.
>
> Here you would simply instantiate dirs/files in securityfs and call the
> existing functions previously introduced.
>
> Roberto
>
I will try to split them in the next version. Thanks for the suggestion.
-Fan
