[prev in list] [next in list] [prev in thread] [next in thread]
List: linux-nfs
Subject: Re: [PATCH] nfs4-acl-tools : nfs4_setfacl' failed with unexpected
From: faizan husain <faizanh () linux ! vnet ! ibm ! com>
Date: 2011-06-30 13:45:26
Message-ID: 4E0C7B26.9030807 () linux ! vnet ! ibm ! com
[Download RAW message or body]
On Thursday 30 June 2011 05:21 PM, Jim Rees wrote:
> faizan husain wrote:
>
> 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?
>
> That looks correct. It should return EINVAL here, and there is no need to
> free. But I don't see why it fixes your segfault. fields[] should be all
> NULL at this point, so free_fields shouldn't do anything.
>
> The test in free_fields() is redundant, since free(NULL) doesn't do
> anything. But it could be made more foolproof by zeroing the array so you
> can't get a double free:
>
> void
> free_fields(char *fields[NUMFIELDS])
> {
> int i;
>
> for (i = 0; i< NUMFIELDS; i++) {
> free(fields[i]);
> fields[i] = NULL;
> }
> }
yeah that could be done to make it more foolproof.
here is the final patch:
From 6cd5263027e3fa5cf18756aa9db108dcdb2367d5 Mon Sep 17 00:00:00 2001
From: faizan <faizan.husain@in.ibm.com>
Date: Thu, 30 Jun 2011 18:55:28 +0530
Subject: [PATCH][BUILD]] FIX - 'nfs4_setfacl' failed with unexpected
messages if
the format of the input file is incorrect.
Signed-off-by: faizan <faizan.husain@in.ibm.com>
---
libnfs4acl/nfs4_ace_from_string.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/libnfs4acl/nfs4_ace_from_string.c
b/libnfs4acl/nfs4_ace_from_string.c
index 9d877fb..b74b1a9 100644
--- a/libnfs4acl/nfs4_ace_from_string.c
+++ b/libnfs4acl/nfs4_ace_from_string.c
@@ -86,9 +86,10 @@ free_fields(char *fields[NUMFIELDS])
{
int i;
- for (i = 0; i < NUMFIELDS; i++)
- if (fields[i] != NULL)
- free(fields[i]);
+ for (i = 0; i < NUMFIELDS; i++) {
+ free(fields[i]);
+ fields[i] = NULL;
+ }
}
int
@@ -107,7 +108,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
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic