Re: [PATCH] nfs4-acl-tools : nfs4_setfacl' failed with unexpected messages if the format of the input file is incorrect.

[faizan husain <faizanh@linux.vnet.ibm.com>]

On Wednesday 29 June 2011 05:48 PM, Jim Rees wrote:
> faizan husain wrote:
>
>    On RHEL5.5 and above releases,
>    read ACL entries to set from file leads to a segmentation fault on pp64,
>    more over the same problem does not show up for x86_64 architecture.
>
> ...
>
>    diff --git a/libnfs4acl/nfs4_ace_from_string.c
>    b/libnfs4acl/nfs4_ace_from_string.c
>    index 9d877fb..1cc220e 100644
>    --- a/libnfs4acl/nfs4_ace_from_string.c
>    +++ b/libnfs4acl/nfs4_ace_from_string.c
>    @@ -125,7 +125,6 @@ parse_alloc_fields(char *buf, char *fields[NUMFIELDS])
>
>            return 0;
>     out_free:
>    -       free_fields(fields);
>            return -ENOMEM;
>     }
>
> If this fix is correct, shouldn't the name of the label be changed?  Better
> yet eliminate the gotos and label.
>
> However, I don't think the fix is correct.  I suspect you need a test for
> strsep() returning NULL.
I have tried strsep() returning NULL but without any success,
have figured out why double free error was coming leading to 
segmentation fault.

problem was this part of code in parse_alloc_fields() function:
if (count != 3)
          goto out_free;
at this point memory is not allocated for fields leading to double free 
of memory once inside parse_alloc_fields() and again inside 
nfs4_ace_from_string().

instead we can change the  code:
if (count != 3)
     return -EINVAL; /*Invalid argument*/

This look to me as more foolproof solution.
what do you say?

Signed-off-by: faizan <faizan.husain@.in.ibm.com>
---
  libnfs4acl/nfs4_ace_from_string.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libnfs4acl/nfs4_ace_from_string.c 
b/libnfs4acl/nfs4_ace_from_string.c
index 9d877fb..6f1e200 100644
--- a/libnfs4acl/nfs4_ace_from_string.c
+++ b/libnfs4acl/nfs4_ace_from_string.c
@@ -107,7 +107,7 @@ parse_alloc_fields(char *buf, char *fields[NUMFIELDS])
                         count++;
         }
         if (count != 3)
-               goto out_free;
+               return -EINVAL;

         for (i = 0; i < NUMFIELDS; i++) {
                 field = strsep(&buf, ":");
--
1.7.1

Thanks
Faizan