On Wed, May 13, 2015 at 01:22:26PM -0400, Benjamin Romer wrote:
> --- a/drivers/staging/unisys/visorbus/visorchipset.c
> +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> @@ -1197,6 +1197,7 @@ bus_create(struct controlvm_message *inmsg)
>       u32 bus_no = cmd->create_bus.bus_no;
>       int rc = CONTROLVM_RESP_SUCCESS;
>       struct visorchipset_bus_info *bus_info;
> +     struct visorchannel *visorchannel;
>  
>       bus_info = bus_find(&bus_info_list, bus_no);
>       if (bus_info && (bus_info->state.created == 1)) {
> @@ -1218,18 +1219,21 @@ bus_create(struct controlvm_message *inmsg)
>  
>       POSTCODE_LINUX_3(BUS_CREATE_ENTRY_PC, bus_no, POSTCODE_SEVERITY_INFO);
>  
> -     if (inmsg->hdr.flags.test_message == 1)
> -             bus_info->chan_info.addr_type = ADDRTYPE_LOCALTEST;
> -     else
> -             bus_info->chan_info.addr_type = ADDRTYPE_LOCALPHYSICAL;
> -
>       bus_info->flags.server = inmsg->hdr.flags.server;
> -     bus_info->chan_info.channel_addr = cmd->create_bus.channel_addr;
> -     bus_info->chan_info.n_channel_bytes = cmd->create_bus.channel_bytes;
> -     bus_info->chan_info.channel_type_uuid =
> -                     cmd->create_bus.bus_data_type_uuid;
> -     bus_info->chan_info.channel_inst_uuid = cmd->create_bus.bus_inst_uuid;
>  
> +     visorchannel = visorchannel_create(cmd->create_bus.channel_addr,
> +                                        cmd->create_bus.channel_bytes,
> +                                        GFP_KERNEL,
> +                                        cmd->create_bus.bus_data_type_uuid);
> +
> +     if (!visorchannel) {
> +             POSTCODE_LINUX_3(BUS_CREATE_FAILURE_PC, bus_no,
> +                              POSTCODE_SEVERITY_ERR);
> +             rc = -CONTROLVM_RESP_ERROR_KMALLOC_FAILED;
> +             kfree(bus_info);


I'm in a very lazy review mood but I can't immediately see how this is
correct.  We're calling kfree(bus_info), but the pointer is still there
and bus_find() will return it to someone else.  Actually it's worse than
that, because bus_find() will dereference it iterating through the
&bus_info_list.

> +             goto cleanup;
> +     }
> +     bus_info->visorchannel = visorchannel;
>       list_add(&bus_info->entry, &bus_info_list);
>  
>       POSTCODE_LINUX_3(BUS_CREATE_EXIT_PC, bus_no, POSTCODE_SEVERITY_INFO);

regards,
dan carpenter
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to