On Mon, Apr 29, 2024 at 11:40 AM Christian Göttsche
<cgoettsche@xxxxxxxxxxxxx> wrote:
>
> From: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
>
> Since commit 5876aca0 ("libselinux: free data on selabel open failure")
> the close handler of label backends must support partial initialized
> state, e.g. ->data being NULL. Thus checks for NULL were added, but in
> two cases the pointers in question were already dereferenced before.
>
> Reorder the dereference after the NULL-checks.
>
> Fixes: 5876aca0 ("libselinux: free data on selabel open failure")
> Reported-by: Cppcheck
> Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> ---
> libselinux/src/label_media.c | 4 +++-
> libselinux/src/label_x.c | 4 +++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libselinux/src/label_media.c b/libselinux/src/label_media.c
> index 94a58062..852aeada 100644
> --- a/libselinux/src/label_media.c
> +++ b/libselinux/src/label_media.c
> @@ -164,12 +164,14 @@ finish:
> static void close(struct selabel_handle *rec)
> {
> struct saved_data *data = (struct saved_data *)rec->data;
> - struct spec *spec, *spec_arr = data->spec_arr;
> + struct spec *spec, *spec_arr;
> unsigned int i;
>
> if (!data)
> return;
>
> + spec_arr = data->spec_arr;
> +
> for (i = 0; i < data->nspec; i++) {
> spec = &spec_arr[i];
> free(spec->key);
> diff --git a/libselinux/src/label_x.c b/libselinux/src/label_x.c
> index f994eefa..a8decc7a 100644
> --- a/libselinux/src/label_x.c
> +++ b/libselinux/src/label_x.c
> @@ -191,12 +191,14 @@ finish:
> static void close(struct selabel_handle *rec)
> {
> struct saved_data *data = (struct saved_data *)rec->data;
> - struct spec *spec, *spec_arr = data->spec_arr;
> + struct spec *spec, *spec_arr;
> unsigned int i;
>
> if (!data)
> return;
>
> + spec_arr = data->spec_arr;
> +
> for (i = 0; i < data->nspec; i++) {
> spec = &spec_arr[i];
> free(spec->key);
> --
> 2.43.0
>
>
LGTM, ack.
