xfs
[Top] [All Lists]

Re: [PATCH] repair: validate acl count before reading it

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] repair: validate acl count before reading it
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 16 Nov 2011 02:58:29 -0500
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20111116002323.GW5534@dastard>
References: <20111115080714.GA24931@xxxxxxxxxxxxx> <20111116002323.GW5534@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Nov 16, 2011 at 11:23:23AM +1100, Dave Chinner wrote:
> >     count = be32_to_cpu(dacl->acl_cnt);
> > +   if (count > XFS_ACL_MAX_ENTRIES) {
> > +           do_warn(_("to larget ACL, size %d"), count);
> 
>                            "Too many ACL entries, count %d\n" 

Ok.

> > +           *aclp = NULL;
> > +           return EINVAL;
> > +   }
> > +
> > +
> >     end = &dacl->acl_entry[0] + count;
> >     acl = malloc((int)((char *)end - (char *)dacl));
> > -   if (!acl)
> > -           return NULL;
> > +   if (!acl) {
> > +           do_warn(_("cannot malloc enough for ACL attribute\n"));
> > +           do_warn(_("SKIPPING this ACL\n"));
> 
> Should you put that same "Skipping" message for all the error cases?

For the ENOMEM case we indeed skip the ACL.  For other errors we will
just remove this attribute.  Given how enomem really should not just
happen for that small ACL I wonder how special casing it makes any
sense - but that code has been there for a while.

<Prev in Thread] Current Thread [Next in Thread>