[Ocfs2-tools-devel] [RFC] PATCH: verify slot number in __ocfs2_read_slot_map(), v2

[Sunil Mushran]

Slot numbers and node numbers are not the same. That comparison is
incorrect. I don't remember the macro offhand. Check out the node
manager code in the kernel.

On Thu, Mar 12, 2009 at 12:47:34AM +0800, Coly Li wrote:
> In some buggy conditions, mounted.ocfs2 does dirty reads, data from
> __ocfs2_read_slot_map() can not be trusted. When it happens, in ocfs2_print_nodes():
>  66                 node_num = map->md_slots[i].sd_node_num;
>  67                 if (names && names[node_num] && *(names[node_num]))
> node_num in 66 may be a very large number (due to the invalid data from
> __ocfs2_read_slot_map()), and names[node_num] references to an illegal memory
> region.
> 
> This patch adds two functions to check whether slot number of each read-in slot
> map item is valid, which can avoid a segmentation fault of mounted.ocfs2 with an
> error message:
> __ocfs2_verify_node_num() for normal slot map item checking
> __ocfs2_verify_node_num_extended() for extended slot map item checking
> 
> Currently, this patch makes mounted.ocfs2 display error message in this style:
> # mounted.ocfs2 -f
> Device                FS     Nodes
> /dev/hdb1             ocfs2  Unknown: Internal logic faliure
> 
> There will be another patch to improve/fix the error message, e.g. from Sunil's
> suggestion, just displaying "Unknow" in Nodes field.
> 
> Signed-off-by: Coly Li <coly.li at suse.de>
> ---
>  libocfs2/slot_map.c |   27 ++++++++++++++++++++++++++-
>  1 files changed, 26 insertions(+), 1 deletions(-)
> 
> diff --git a/libocfs2/slot_map.c b/libocfs2/slot_map.c
> index c33f458..e8d22a1 100644
> --- a/libocfs2/slot_map.c
> +++ b/libocfs2/slot_map.c
> @@ -54,6 +54,29 @@ void ocfs2_swap_slot_map_extended(struct
> ocfs2_slot_map_extended *se,
>  			bswap_32(se->se_slots[i].es_node_num);
>  }
> 
> +/* es_node_num should be swapped to local cpu endian */
> +static errcode_t __ocfs2_verify_node_num(struct ocfs2_slot_map *sm,
> +					int num_slots)
> +{
> +	int i;
> +
> +	for (i = 0; i < num_slots; i++)
> +		if (sm->sm_slots[i] > OCFS2_MAX_SLOTS)
> +			return OCFS2_ET_INTERNAL_FAILURE;
> +	return 0;
> +}
> +
> +/* es_node_num should be swapped to local cpu endian */
> +static errcode_t __ocfs2_verify_node_num_extended(struct
> ocfs2_slot_map_extended *se,
> +						int num_slots)
> +{
> +	int i;
> +	for (i = 0; i < num_slots; i++)
> +		if (se->se_slots[i].es_node_num > OCFS2_MAX_SLOTS)
> +			return OCFS2_ET_INTERNAL_FAILURE;
> +	return 0;
> +}
> +
>  static errcode_t __ocfs2_read_slot_map(ocfs2_filesys *fs,
>  				       int num_slots,
>  				       union ocfs2_slot_map_wrapper *wrap)
> @@ -90,13 +113,15 @@ static errcode_t __ocfs2_read_slot_map(ocfs2_filesys *fs,
>  		se = (struct ocfs2_slot_map_extended *)slot_map_buf;
>  		ocfs2_swap_slot_map_extended(se, num_slots);
>  		wrap->mw_map_extended = se;
> +		ret = __ocfs2_verify_node_num_extended(se, num_slots);
>  	} else {
>  		sm = (struct ocfs2_slot_map *)slot_map_buf;
>  		ocfs2_swap_slot_map(sm, num_slots);
>  		wrap->mw_map = sm;
> +		ret = __ocfs2_verify_node_num(sm, num_slots);
>  	}
> 
> -	return 0;
> +	return ret;
>  }
> 
>  errcode_t ocfs2_read_slot_map(ocfs2_filesys *fs,
> 
> -- 
> Coly Li
> SuSE Labs
> 
> _______________________________________________
> Ocfs2-tools-devel mailing list
> Ocfs2-tools-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-tools-devel