[Gluster-devel] [PATCH BUG:493] Fix memory access in afr's self-heal code (replace pointer casts by memcpy).

Raghavendra G raghavendra at gluster.com
Mon Dec 21 17:38:32 UTC 2009


Hi Hraban,

If you are resubmitting the patches, please make sure to mark the patch as
"superseded" at patches.gluster.com. It will help the maintainer :).

regards,
On Mon, Dec 21, 2009 at 10:59 AM, Hraban Luyat <hraban at 0brg.net> wrote:

> The previous patch I submitted for this file (afr's self-heal code)
> introduced a terrible error. I overlooked the error checking in the
> original code and misplaced the memcpy (too early).
>
> So, please disregard the last one, this one is better :) hopefully.
>
> Apologies.
>
> Signed-off-by: Hraban Luyat <hraban at 0brg.net>
> ---
>  xlators/cluster/afr/src/afr-self-heal-common.c |   54
> +++++++++++++----------
>  1 files changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c
> b/xlators/cluster/afr/src/afr-self-heal-common.c
> index ef36be1..61c3d2b 100644
> --- a/xlators/cluster/afr/src/afr-self-heal-common.c
> +++ b/xlators/cluster/afr/src/afr-self-heal-common.c
> @@ -122,7 +122,9 @@ afr_sh_build_pending_matrix (afr_private_t *priv,
>  {
>        int i, j, k;
>
> -       int32_t *pending = NULL;
> +       /* Indexable by result of afr_index_for_transaction_type(): 0 -- 2.
> */
> +       int32_t pending[3];
> +       void *pending_raw = NULL;
>        int ret = -1;
>
>         unsigned char *ignorant_subvols = NULL;
> @@ -137,11 +139,11 @@ afr_sh_build_pending_matrix (afr_private_t *priv,
>        }
>
>        for (i = 0; i < child_count; i++) {
> -               pending = NULL;
> +               pending_raw = NULL;
>
>                 for (j = 0; j < child_count; j++) {
>                         ret = dict_get_ptr (xattr[i], priv->pending_key[j],
> -                                            VOID(&pending));
> +                                            &pending_raw);
>
>                         if (ret != 0) {
>                                 /*
> @@ -154,6 +156,7 @@ afr_sh_build_pending_matrix (afr_private_t *priv,
>                                 continue;
>                         }
>
> +                       memcpy (pending, pending_raw, sizeof(pending));
>                         k = afr_index_for_transaction_type (type);
>
>                         pending_matrix[i][j] = ntoh32 (pending[k]);
> @@ -527,8 +530,10 @@ afr_sh_pending_to_delta (afr_private_t *priv, dict_t
> **xattr,
>        int j = 0;
>         int k = 0;
>
> -        int32_t * pending = NULL;
> -        int       ret     = 0;
> +       /* Indexable by result of afr_index_for_transaction_type(): 0 -- 2.
> */
> +       int32_t   pending[3];
> +       void    * pending_raw = NULL;
> +        int       ret         = 0;
>
>        /* start clean */
>        for (i = 0; i < child_count; i++) {
> @@ -538,18 +543,19 @@ afr_sh_pending_to_delta (afr_private_t *priv, dict_t
> **xattr,
>        }
>
>        for (i = 0; i < child_count; i++) {
> -                pending = NULL;
> +                pending_raw = NULL;
>
>                 for (j = 0; j < child_count; j++) {
>                         ret = dict_get_ptr (xattr[i], priv->pending_key[j],
> -                                            VOID(&pending));
> -
> +                                            &pending_raw);
> +
>                         if (!success[j])
>                                 continue;
>
>                         k = afr_index_for_transaction_type (type);
>
> -                        if (pending) {
> +                        if (pending_raw) {
> +                               memcpy (pending, pending_raw,
> sizeof(pending));
>                                 delta_matrix[i][j] = -(ntoh32
> (pending[k]));
>                         } else {
>                                 delta_matrix[i][j]  = 0;
> @@ -599,8 +605,9 @@ int
>  afr_sh_has_metadata_pending (dict_t *xattr, int child_count, xlator_t
> *this)
>  {
>        afr_private_t *priv = NULL;
> -       int32_t       *pending = NULL;
> -       void          *tmp_pending = NULL; /* This is required to remove
> 'type-punned' warnings from gcc */
> +       /* Indexable by result of afr_index_for_transaction_type(): 0 -- 2.
> */
> +       int32_t       pending[3];
> +       void          *pending_raw = NULL;
>
>        int           ret = -1;
>        int            i  = 0;
> @@ -610,13 +617,12 @@ afr_sh_has_metadata_pending (dict_t *xattr, int
> child_count, xlator_t *this)
>
>         for (i = 0; i < priv->child_count; i++) {
>                 ret = dict_get_ptr (xattr, priv->pending_key[i],
> -                                    &tmp_pending);
> +                                    &pending_raw);
>
>                 if (ret != 0)
>                         return 0;
> -
> -                pending = tmp_pending;
>
> +               memcpy (pending, pending_raw, sizeof(pending));
>                 j = afr_index_for_transaction_type
> (AFR_METADATA_TRANSACTION);
>
>                 if (pending[j])
> @@ -631,8 +637,9 @@ int
>  afr_sh_has_data_pending (dict_t *xattr, int child_count, xlator_t *this)
>  {
>        afr_private_t *priv = NULL;
> -       int32_t       *pending = NULL;
> -       void          *tmp_pending = NULL; /* This is required to remove
> 'type-punned' warnings from gcc */
> +       /* Indexable by result of afr_index_for_transaction_type(): 0 -- 2.
> */
> +       int32_t       pending[3];
> +       void          *pending_raw = NULL;
>
>        int           ret = -1;
>        int            i  = 0;
> @@ -642,13 +649,12 @@ afr_sh_has_data_pending (dict_t *xattr, int
> child_count, xlator_t *this)
>
>         for (i = 0; i < priv->child_count; i++) {
>                 ret = dict_get_ptr (xattr, priv->pending_key[i],
> -                                    &tmp_pending);
> +                                    &pending_raw);
>
>                 if (ret != 0)
>                         return 0;
>
> -                pending = tmp_pending;
> -
> +               memcpy (pending, pending_raw, sizeof(pending));
>                 j = afr_index_for_transaction_type (AFR_DATA_TRANSACTION);
>
>                 if (pending[j])
> @@ -663,8 +669,9 @@ int
>  afr_sh_has_entry_pending (dict_t *xattr, int child_count, xlator_t *this)
>  {
>         afr_private_t *priv = NULL;
> -       int32_t       *pending = NULL;
> -       void          *tmp_pending = NULL; /* This is required to remove
> 'type-punned' warnings from gcc */
> +       /* Indexable by result of afr_index_for_transaction_type(): 0 -- 2.
> */
> +       int32_t       pending[3];
> +       void          *pending_raw = NULL;
>
>        int           ret = -1;
>        int            i  = 0;
> @@ -674,13 +681,12 @@ afr_sh_has_entry_pending (dict_t *xattr, int
> child_count, xlator_t *this)
>
>         for (i = 0; i < priv->child_count; i++) {
>                 ret = dict_get_ptr (xattr, priv->pending_key[i],
> -                                    &tmp_pending);
> +                                    &pending_raw);
>
>                 if (ret != 0)
>                         return 0;
>
> -                pending = tmp_pending;
> -
> +               memcpy (pending, pending_raw, sizeof(pending));
>                 j = afr_index_for_transaction_type (AFR_ENTRY_TRANSACTION);
>
>                 if (pending[j])
> --
> 1.6.5
>
>
>
> _______________________________________________
> Gluster-devel mailing list
> Gluster-devel at nongnu.org
> http://lists.nongnu.org/mailman/listinfo/gluster-devel
>



-- 
Raghavendra G
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://supercolony.gluster.org/pipermail/gluster-devel/attachments/20091221/287613df/attachment-0003.html>


More information about the Gluster-devel mailing list