Hi,

The Coverity checker points out (CID: 1500) that we can in some 
cases end up doing a double free of 'model' in asus_hotk_get_info().
I'm not 100% sure it is right, but better safe than sorry, 
especially since this is so simple to turn into a non-issue - simply 
set 'model' to NULL after the first kfree() and then the second 
kfree() is harmless (if it actually can happen, and if it cannot
happen then the cost is just a single extra assignment).

Here is the function with Coverity's annotations 
(proposed patch at the end of the mail)

...
1141    /*
1142     * This function is used to initialize the hotk with right values. In 
this
1143     * method, we can make all the detection we want, and modify the hotk 
struct
1144     */
1145    static int asus_hotk_get_info(void)
1146    {
1147            struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
1148            union acpi_object *model = NULL;
1149            int bsts_result;
1150            char *string = NULL;
1151            acpi_status status;
1152    
1153            /*
1154             * Get DSDT headers early enough to allow for differentiating 
between 
1155             * models, but late enough to allow acpi_bus_register_driver() 
to fail 
1156             * before doing anything ACPI-specific. Should we encounter a 
machine,
1157             * which needs special handling (i.e. its hotkey device has a 
different
1158             * HID), this bit will be moved. A global variable asus_info 
contains
1159             * the DSDT header.
1160             */
1161            status = acpi_get_table(ACPI_SIG_DSDT, 1, &asus_info);
1162            if (ACPI_FAILURE(status))
1163                    printk(KERN_WARNING "  Couldn't get the DSDT table 
header\n");
1164    
1165            /* We have to write 0 on init this far for all ASUS models */
1166            if (!write_acpi_int(hotk->handle, "INIT", 0, &buffer)) {
1167                    printk(KERN_ERR "  Hotkey initialization failed\n");
1168                    return -ENODEV;
1169            }
1170    
1171            /* This needs to be called for some laptops to init properly */
1172            if (!read_acpi_int(hotk->handle, "BSTS", &bsts_result))
1173                    printk(KERN_WARNING "  Error calling BSTS\n");
1174            else if (bsts_result)
1175                    printk(KERN_NOTICE "  BSTS called, 0x%02x returned\n",
1176                           bsts_result);
1177    
1178            /*
1179             * Try to match the object returned by INIT to the specific 
model.
1180             * Handle every possible object (or the lack of thereof) the 
DSDT 
1181             * writers might throw at us. When in trouble, we pass NULL to 
1182             * asus_model_match() and try something completely different.
1183             */
1184            if (buffer.pointer) {

Event alias: aliasing "(buffer).pointer" with "model"
Also see events: [freed_arg][double_free]

1185                    model = buffer.pointer;
1186                    switch (model->type) {
1187                    case ACPI_TYPE_STRING:
1188                            string = model->string.pointer;
1189                            break;
1190                    case ACPI_TYPE_BUFFER:
1191                            string = model->buffer.pointer;
1192                            break;

At conditional (1): "default" taking true path

1193                    default:

Event freed_arg: Pointer "model" freed by function "kfree"
Also see events: [alias][double_free]

1194                            kfree(model);
1195                            break;
1196                    }
1197            }
1198            hotk->model = asus_model_match(string);

At conditional (2): "(hotk)->model == 23" taking false path

1199            if (hotk->model == END_MODEL) { /* match failed */
1200                    if (asus_info &&
1201                        strncmp(asus_info->oem_table_id, "ODEM", 4) == 0) {
1202                            hotk->model = P30;
1203                            printk(KERN_NOTICE
1204                                   "  Samsung P30 detected, supported\n");
1205                    } else {
1206                            hotk->model = M2E;
1207                            printk(KERN_NOTICE "  unsupported model %s, 
trying "
1208                                   "default values\n", string);
1209                            printk(KERN_NOTICE
1210                                   "  send /proc/acpi/dsdt to the 
developers\n");
1211                    }
1212                    hotk->methods = &model_conf[hotk->model];
1213                    return AE_OK;
1214            }
1215            hotk->methods = &model_conf[hotk->model];
1216            printk(KERN_NOTICE "  %s model detected, supported\n", string);
1217    
1218            /* Sort of per-model blacklist */

At conditional (3): "strncmp == 0" taking false path

1219            if (strncmp(string, "L2B", 3) == 0)
1220                    hotk->methods->lcd_status = NULL;
1221            /* L2B is similar enough to L3C to use its settings, with this 
only 
1222               exception */

At conditional (4): "strncmp == 0" taking false path

1223            else if (strncmp(string, "A3G", 3) == 0)
1224                    hotk->methods->lcd_status = "\\BLFG";
1225            /* A3G is like M6R */

At conditional (5): "strncmp == 0" taking false path
At conditional (6): "strncmp == 0" taking false path
At conditional (7): "strncmp == 0" taking false path

1226            else if (strncmp(string, "S5N", 3) == 0 ||
1227                     strncmp(string, "M5N", 3) == 0 ||
1228                     strncmp(string, "W3N", 3) == 0)
1229                    hotk->methods->mt_mled = NULL;
1230            /* S5N, M5N and W3N have no MLED */

At conditional (8): "strncmp == 0" taking false path

1231            else if (strncmp(string, "L5D", 3) == 0)
1232                    hotk->methods->mt_wled = NULL;
1233            /* L5D's WLED is not controlled by ACPI */

At conditional (9): "strncmp == 0" taking false path
At conditional (10): "strncmp == 0" taking false path
At conditional (11): "strncmp == 0" taking false path

1234            else if (strncmp(string, "M2N", 3) == 0 ||
1235                     strncmp(string, "W3V", 3) == 0 ||
1236                     strncmp(string, "S1N", 3) == 0)
1237                    hotk->methods->mt_wled = "WLED";
1238            /* M2N, S1N and W3V have a usable WLED */

At conditional (12): "asus_info != 0" taking true path

1239            else if (asus_info) {

At conditional (13): "strncmp == 0" taking false path

1240                    if (strncmp(asus_info->oem_table_id, "L1", 2) == 0)
1241                            hotk->methods->mled_status = NULL;
1242                    /* S1300A reports L84F, but L1400B too, account for 
that */
1243            }
1244    

Event double_free: Double free of pointer "model" in call to "kfree"
Also see events: [alias][freed_arg]

1245            kfree(model);
1246    
1247            return AE_OK;
1248    }
...




Signed-off-by: Jesper Juhl <[EMAIL PROTECTED]>
---

 drivers/acpi/asus_acpi.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/asus_acpi.c b/drivers/acpi/asus_acpi.c
index 9c4bd22..86fd142 100644
--- a/drivers/acpi/asus_acpi.c
+++ b/drivers/acpi/asus_acpi.c
@@ -1192,6 +1192,7 @@ static int asus_hotk_get_info(void)
                        break;
                default:
                        kfree(model);
+                       model = NULL;
                        break;
                }
        }




Please Cc me on replies...


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to