Commit e533720df36dec3dba13198f7481827cb511fefc
Committed by
Jyri Sarha
1 parent
f6d0086fa6
Exists in
smarc-ti-linux-3.14.y
and in
1 other branch
media: ti-vpe: vpdma/vip/vpe: Fix race condition for firmware loading
vpdma_create API is supposed to allocated the struct vpdma_data and return it to the driver. Also, it would call the callback function when the VPDMA firmware is loaded. Typically, VIP VPE drivers have following function call: dev->vpdma = vpdma_create(pdev, firmware_load_callback); And the callback implementation would continue the probe further. Also, the dev->vpdma is accessed from the callback implementation. This may lead to race condition between assignment of dev->vpdma and the callback function being triggered. This would lead to kernel crash because of NULL pointer access. Fix this by passing a driver wrapped &vpdma_data instead of allocating inside vpdma_create. Change the vpdma_create prototype accordingly and fix return paths. Also, update the VIP and VPE driver to use the updated API and initialize the dev->vpdma before hand so that the race condition is avoided. Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com> Signed-off-by: Jyri Sarha <jsarha@ti.com>
Showing 5 changed files with 14 additions and 19 deletions Side-by-side Diff
drivers/media/platform/ti-vpe/vip.c
... | ... | @@ -2943,8 +2943,9 @@ |
2943 | 2943 | vip_set_slice_path(dev, VIP_MULTI_CHANNEL_DATA_SELECT); |
2944 | 2944 | } |
2945 | 2945 | |
2946 | - shared->vpdma = vpdma_create(pdev, vip_vpdma_fw_cb); | |
2947 | - if (!shared->vpdma) { | |
2946 | + shared->vpdma = &shared->vpdma_data; | |
2947 | + ret = vpdma_create(pdev, shared->vpdma, vip_vpdma_fw_cb); | |
2948 | + if (ret) { | |
2948 | 2949 | dev_err(&pdev->dev, "Creating VPDMA failed"); |
2949 | 2950 | goto dev_unreg; |
2950 | 2951 | } |
drivers/media/platform/ti-vpe/vip.h
drivers/media/platform/ti-vpe/vpdma.c
... | ... | @@ -1129,21 +1129,14 @@ |
1129 | 1129 | return 0; |
1130 | 1130 | } |
1131 | 1131 | |
1132 | -struct vpdma_data *vpdma_create(struct platform_device *pdev, | |
1132 | +int vpdma_create(struct platform_device *pdev, struct vpdma_data *vpdma, | |
1133 | 1133 | void (*cb)(struct platform_device *pdev)) |
1134 | 1134 | { |
1135 | 1135 | struct resource *res; |
1136 | - struct vpdma_data *vpdma; | |
1137 | 1136 | int r; |
1138 | 1137 | |
1139 | 1138 | dev_dbg(&pdev->dev, "vpdma_create\n"); |
1140 | 1139 | |
1141 | - vpdma = devm_kzalloc(&pdev->dev, sizeof(*vpdma), GFP_KERNEL); | |
1142 | - if (!vpdma) { | |
1143 | - dev_err(&pdev->dev, "couldn't alloc vpdma_dev\n"); | |
1144 | - return ERR_PTR(-ENOMEM); | |
1145 | - } | |
1146 | - | |
1147 | 1140 | vpdma->pdev = pdev; |
1148 | 1141 | vpdma->cb = cb; |
1149 | 1142 | spin_lock_init(&vpdma->lock); |
1150 | 1143 | |
1151 | 1144 | |
1152 | 1145 | |
... | ... | @@ -1151,22 +1144,22 @@ |
1151 | 1144 | res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "vpdma"); |
1152 | 1145 | if (res == NULL) { |
1153 | 1146 | dev_err(&pdev->dev, "missing platform resources data\n"); |
1154 | - return ERR_PTR(-ENODEV); | |
1147 | + return -ENODEV; | |
1155 | 1148 | } |
1156 | 1149 | |
1157 | 1150 | vpdma->base = devm_ioremap(&pdev->dev, res->start, resource_size(res)); |
1158 | 1151 | if (!vpdma->base) { |
1159 | 1152 | dev_err(&pdev->dev, "failed to ioremap\n"); |
1160 | - return ERR_PTR(-ENOMEM); | |
1153 | + return -ENOMEM; | |
1161 | 1154 | } |
1162 | 1155 | |
1163 | 1156 | r = vpdma_load_firmware(vpdma); |
1164 | 1157 | if (r) { |
1165 | 1158 | pr_err("failed to load firmware %s\n", VPDMA_FIRMWARE); |
1166 | - return ERR_PTR(r); | |
1159 | + return r; | |
1167 | 1160 | } |
1168 | 1161 | |
1169 | - return vpdma; | |
1162 | + return 0; | |
1170 | 1163 | } |
1171 | 1164 | EXPORT_SYMBOL(vpdma_create); |
1172 | 1165 |
drivers/media/platform/ti-vpe/vpdma.h
... | ... | @@ -273,7 +273,7 @@ |
273 | 273 | void vpdma_dump_regs(struct vpdma_data *vpdma); |
274 | 274 | |
275 | 275 | /* initialize vpdma, passed with VPE's platform device pointer */ |
276 | -struct vpdma_data *vpdma_create(struct platform_device *pdev, | |
276 | +int vpdma_create(struct platform_device *pdev, struct vpdma_data *vpdma, | |
277 | 277 | void (*cb)(struct platform_device *pdev)); |
278 | 278 | |
279 | 279 | #endif |
drivers/media/platform/ti-vpe/vpe.c
... | ... | @@ -381,6 +381,7 @@ |
381 | 381 | struct resource *res; |
382 | 382 | |
383 | 383 | struct vb2_alloc_ctx *alloc_ctx; |
384 | + struct vpdma_data vpdma_data; | |
384 | 385 | struct vpdma_data *vpdma; /* vpdma data handle */ |
385 | 386 | struct sc_data *sc; /* scaler data handle */ |
386 | 387 | struct csc_data *csc; /* csc data handle */ |
387 | 388 | |
... | ... | @@ -2481,11 +2482,10 @@ |
2481 | 2482 | goto runtime_put; |
2482 | 2483 | } |
2483 | 2484 | |
2484 | - dev->vpdma = vpdma_create(pdev, vpe_fw_cb); | |
2485 | - if (IS_ERR(dev->vpdma)) { | |
2486 | - ret = PTR_ERR(dev->vpdma); | |
2485 | + dev->vpdma = &dev->vpdma_data; | |
2486 | + ret = vpdma_create(pdev, dev->vpdma, vpe_fw_cb); | |
2487 | + if (ret) | |
2487 | 2488 | goto runtime_put; |
2488 | - } | |
2489 | 2489 | |
2490 | 2490 | return 0; |
2491 | 2491 |