Skip to contentSkip to conversation

of: overlay: rework overlay apply and remove kfree()s

bug report Dan Carpenter 2 participants 2 messages Apr 27, 2022 – Apr 28, 2022

lore
Share & tools
lore
#0 lore raw

Hello Frank Rowand,

The patch 067c098766c6: "of: overlay: rework overlay apply and remove kfree()s" from Apr 20, 2022, leads to the following Smatch static checker warning:

drivers/of/overlay.c:180 overlay_notify()
error: buffer overflow 'of_overlay_action_name' 4 <= 4

drivers/of/overlay.c
155 static char *of_overlay_action_name[] = {

We need to add a "init" string at the start of this array.

   156          "pre-apply",
   157          "post-apply",
   158          "pre-remove",
   159          "post-remove",
   160  };
   161  
    162 static int overlay_notify(struct overlay_changeset *ovcs,
    163                 enum of_overlay_notify_action action)
    164 {
    165         struct of_overlay_notify_data nd;
    166         int i, ret;
    167 
    168         ovcs->notify_state = action;
    169 
    170         for (i = 0; i < ovcs->count; i++) {
    171                 struct fragment *fragment = &ovcs->fragments[i];
    172 
    173                 nd.target = fragment->target;
    174                 nd.overlay = fragment->overlay;
    175 
    176                 ret = blocking_notifier_call_chain(&overlay_notify_chain,
    177                                                    action, &nd);
    178                 if (notifier_to_errno(ret)) {
    179                         ret = notifier_to_errno(ret);
--> 180                         pr_err("overlay changeset %s notifier error %d, target: %pOF\n",
    181                                of_overlay_action_name[action], ret, nd.target);
                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

These are all shifted and so post remove is out of bounds.

    182                         return ret;
    183                 }
    184         }
    185 
    186         return 0;
    187 }

regards,
dan carpenter

#1 lore raw
Quoted 50 lines · Dan Carpenter

Hello Frank Rowand,

The patch 067c098766c6: "of: overlay: rework overlay apply and remove kfree()s" from Apr 20, 2022, leads to the following Smatch static checker warning:

drivers/of/overlay.c:180 overlay_notify()
error: buffer overflow 'of_overlay_action_name' 4 <= 4

drivers/of/overlay.c
155 static char *of_overlay_action_name[] = {

We need to add a "init" string at the start of this array.

   156          "pre-apply",
   157          "post-apply",
   158          "pre-remove",
   159          "post-remove",
   160  };
   161  
    162 static int overlay_notify(struct overlay_changeset *ovcs,
    163                 enum of_overlay_notify_action action)
    164 {
    165         struct of_overlay_notify_data nd;
    166         int i, ret;
    167 
    168         ovcs->notify_state = action;
    169 
    170         for (i = 0; i < ovcs->count; i++) {
    171                 struct fragment *fragment = &ovcs->fragments[i];
    172 
    173                 nd.target = fragment->target;
    174                 nd.overlay = fragment->overlay;
    175 
    176                 ret = blocking_notifier_call_chain(&overlay_notify_chain,
    177                                                    action, &nd);
    178                 if (notifier_to_errno(ret)) {
    179                         ret = notifier_to_errno(ret);
--> 180                         pr_err("overlay changeset %s notifier error %d, target: %pOF\n",
    181                                of_overlay_action_name[action], ret, nd.target);
                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

These are all shifted and so post remove is out of bounds.

    182                         return ret;
    183                 }
    184         }
    185 
    186         return 0;
    187 }
Quoted 2 lines

regards,
dan carpenter

Thanks for the bug report!

I'm working on a patch to fix (easy) and a patch to add some unittest tests to verify the overlay notifiers (a little bit more time consuming).

-Frank