Skip to content

Commit ae7df8f

Browse files
committed
Merge tag 'char-misc-4.14-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc
Pull char/misc driver fixes from Greg KH: "Here are 4 patches to resolve some char/misc driver issues found these past weeks. One of them is a mei bugfix and another is a new mei device id. There is also a hyper-v fix for a reported issue, and a binder issue fix for a problem reported by a few people. All of these have been in my tree for a while, I don't know if linux-next is really testing much this month. But 0-day is happy with them :)" * tag 'char-misc-4.14-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc: binder: fix use-after-free in binder_transaction() Drivers: hv: vmbus: Fix bugs in rescind handling mei: me: add gemini lake devices id mei: always use domain runtime pm callbacks.
2 parents 7a263b1 + 512cf46 commit ae7df8f

File tree

8 files changed

+115
-81
lines changed

8 files changed

+115
-81
lines changed

drivers/android/binder.c

Lines changed: 66 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2582,6 +2582,48 @@ static bool binder_proc_transaction(struct binder_transaction *t,
25822582
return true;
25832583
}
25842584

2585+
/**
2586+
* binder_get_node_refs_for_txn() - Get required refs on node for txn
2587+
* @node: struct binder_node for which to get refs
2588+
* @proc: returns @node->proc if valid
2589+
* @error: if no @proc then returns BR_DEAD_REPLY
2590+
*
2591+
* User-space normally keeps the node alive when creating a transaction
2592+
* since it has a reference to the target. The local strong ref keeps it
2593+
* alive if the sending process dies before the target process processes
2594+
* the transaction. If the source process is malicious or has a reference
2595+
* counting bug, relying on the local strong ref can fail.
2596+
*
2597+
* Since user-space can cause the local strong ref to go away, we also take
2598+
* a tmpref on the node to ensure it survives while we are constructing
2599+
* the transaction. We also need a tmpref on the proc while we are
2600+
* constructing the transaction, so we take that here as well.
2601+
*
2602+
* Return: The target_node with refs taken or NULL if no @node->proc is NULL.
2603+
* Also sets @proc if valid. If the @node->proc is NULL indicating that the
2604+
* target proc has died, @error is set to BR_DEAD_REPLY
2605+
*/
2606+
static struct binder_node *binder_get_node_refs_for_txn(
2607+
struct binder_node *node,
2608+
struct binder_proc **procp,
2609+
uint32_t *error)
2610+
{
2611+
struct binder_node *target_node = NULL;
2612+
2613+
binder_node_inner_lock(node);
2614+
if (node->proc) {
2615+
target_node = node;
2616+
binder_inc_node_nilocked(node, 1, 0, NULL);
2617+
binder_inc_node_tmpref_ilocked(node);
2618+
node->proc->tmp_ref++;
2619+
*procp = node->proc;
2620+
} else
2621+
*error = BR_DEAD_REPLY;
2622+
binder_node_inner_unlock(node);
2623+
2624+
return target_node;
2625+
}
2626+
25852627
static void binder_transaction(struct binder_proc *proc,
25862628
struct binder_thread *thread,
25872629
struct binder_transaction_data *tr, int reply,
@@ -2685,43 +2727,35 @@ static void binder_transaction(struct binder_proc *proc,
26852727
ref = binder_get_ref_olocked(proc, tr->target.handle,
26862728
true);
26872729
if (ref) {
2688-
binder_inc_node(ref->node, 1, 0, NULL);
2689-
target_node = ref->node;
2690-
}
2691-
binder_proc_unlock(proc);
2692-
if (target_node == NULL) {
2730+
target_node = binder_get_node_refs_for_txn(
2731+
ref->node, &target_proc,
2732+
&return_error);
2733+
} else {
26932734
binder_user_error("%d:%d got transaction to invalid handle\n",
2694-
proc->pid, thread->pid);
2735+
proc->pid, thread->pid);
26952736
return_error = BR_FAILED_REPLY;
2696-
return_error_param = -EINVAL;
2697-
return_error_line = __LINE__;
2698-
goto err_invalid_target_handle;
26992737
}
2738+
binder_proc_unlock(proc);
27002739
} else {
27012740
mutex_lock(&context->context_mgr_node_lock);
27022741
target_node = context->binder_context_mgr_node;
2703-
if (target_node == NULL) {
2742+
if (target_node)
2743+
target_node = binder_get_node_refs_for_txn(
2744+
target_node, &target_proc,
2745+
&return_error);
2746+
else
27042747
return_error = BR_DEAD_REPLY;
2705-
mutex_unlock(&context->context_mgr_node_lock);
2706-
return_error_line = __LINE__;
2707-
goto err_no_context_mgr_node;
2708-
}
2709-
binder_inc_node(target_node, 1, 0, NULL);
27102748
mutex_unlock(&context->context_mgr_node_lock);
27112749
}
2712-
e->to_node = target_node->debug_id;
2713-
binder_node_lock(target_node);
2714-
target_proc = target_node->proc;
2715-
if (target_proc == NULL) {
2716-
binder_node_unlock(target_node);
2717-
return_error = BR_DEAD_REPLY;
2750+
if (!target_node) {
2751+
/*
2752+
* return_error is set above
2753+
*/
2754+
return_error_param = -EINVAL;
27182755
return_error_line = __LINE__;
27192756
goto err_dead_binder;
27202757
}
2721-
binder_inner_proc_lock(target_proc);
2722-
target_proc->tmp_ref++;
2723-
binder_inner_proc_unlock(target_proc);
2724-
binder_node_unlock(target_node);
2758+
e->to_node = target_node->debug_id;
27252759
if (security_binder_transaction(proc->tsk,
27262760
target_proc->tsk) < 0) {
27272761
return_error = BR_FAILED_REPLY;
@@ -3071,6 +3105,8 @@ static void binder_transaction(struct binder_proc *proc,
30713105
if (target_thread)
30723106
binder_thread_dec_tmpref(target_thread);
30733107
binder_proc_dec_tmpref(target_proc);
3108+
if (target_node)
3109+
binder_dec_node_tmpref(target_node);
30743110
/*
30753111
* write barrier to synchronize with initialization
30763112
* of log entry
@@ -3090,6 +3126,8 @@ static void binder_transaction(struct binder_proc *proc,
30903126
err_copy_data_failed:
30913127
trace_binder_transaction_failed_buffer_release(t->buffer);
30923128
binder_transaction_buffer_release(target_proc, t->buffer, offp);
3129+
if (target_node)
3130+
binder_dec_node_tmpref(target_node);
30933131
target_node = NULL;
30943132
t->buffer->transaction = NULL;
30953133
binder_alloc_free_buf(&target_proc->alloc, t->buffer);
@@ -3104,13 +3142,14 @@ static void binder_transaction(struct binder_proc *proc,
31043142
err_empty_call_stack:
31053143
err_dead_binder:
31063144
err_invalid_target_handle:
3107-
err_no_context_mgr_node:
31083145
if (target_thread)
31093146
binder_thread_dec_tmpref(target_thread);
31103147
if (target_proc)
31113148
binder_proc_dec_tmpref(target_proc);
3112-
if (target_node)
3149+
if (target_node) {
31133150
binder_dec_node(target_node, 1, 0);
3151+
binder_dec_node_tmpref(target_node);
3152+
}
31143153

31153154
binder_debug(BINDER_DEBUG_FAILED_TRANSACTION,
31163155
"%d:%d transaction failed %d/%d, size %lld-%lld line %d\n",

drivers/hv/channel.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,7 @@ void vmbus_close(struct vmbus_channel *channel)
640640
*/
641641
return;
642642
}
643+
mutex_lock(&vmbus_connection.channel_mutex);
643644
/*
644645
* Close all the sub-channels first and then close the
645646
* primary channel.
@@ -648,16 +649,15 @@ void vmbus_close(struct vmbus_channel *channel)
648649
cur_channel = list_entry(cur, struct vmbus_channel, sc_list);
649650
vmbus_close_internal(cur_channel);
650651
if (cur_channel->rescind) {
651-
mutex_lock(&vmbus_connection.channel_mutex);
652-
hv_process_channel_removal(cur_channel,
652+
hv_process_channel_removal(
653653
cur_channel->offermsg.child_relid);
654-
mutex_unlock(&vmbus_connection.channel_mutex);
655654
}
656655
}
657656
/*
658657
* Now close the primary.
659658
*/
660659
vmbus_close_internal(channel);
660+
mutex_unlock(&vmbus_connection.channel_mutex);
661661
}
662662
EXPORT_SYMBOL_GPL(vmbus_close);
663663

drivers/hv/channel_mgmt.c

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ static void vmbus_rescind_cleanup(struct vmbus_channel *channel)
159159

160160

161161
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
162-
162+
channel->rescind = true;
163163
list_for_each_entry(msginfo, &vmbus_connection.chn_msg_list,
164164
msglistentry) {
165165

@@ -381,14 +381,21 @@ static void vmbus_release_relid(u32 relid)
381381
true);
382382
}
383383

384-
void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
384+
void hv_process_channel_removal(u32 relid)
385385
{
386386
unsigned long flags;
387-
struct vmbus_channel *primary_channel;
387+
struct vmbus_channel *primary_channel, *channel;
388388

389-
BUG_ON(!channel->rescind);
390389
BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
391390

391+
/*
392+
* Make sure channel is valid as we may have raced.
393+
*/
394+
channel = relid2channel(relid);
395+
if (!channel)
396+
return;
397+
398+
BUG_ON(!channel->rescind);
392399
if (channel->target_cpu != get_cpu()) {
393400
put_cpu();
394401
smp_call_function_single(channel->target_cpu,
@@ -515,6 +522,7 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
515522
if (!fnew) {
516523
if (channel->sc_creation_callback != NULL)
517524
channel->sc_creation_callback(newchannel);
525+
newchannel->probe_done = true;
518526
return;
519527
}
520528

@@ -834,7 +842,6 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
834842
{
835843
struct vmbus_channel_rescind_offer *rescind;
836844
struct vmbus_channel *channel;
837-
unsigned long flags;
838845
struct device *dev;
839846

840847
rescind = (struct vmbus_channel_rescind_offer *)hdr;
@@ -873,16 +880,6 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
873880
return;
874881
}
875882

876-
spin_lock_irqsave(&channel->lock, flags);
877-
channel->rescind = true;
878-
spin_unlock_irqrestore(&channel->lock, flags);
879-
880-
/*
881-
* Now that we have posted the rescind state, perform
882-
* rescind related cleanup.
883-
*/
884-
vmbus_rescind_cleanup(channel);
885-
886883
/*
887884
* Now wait for offer handling to complete.
888885
*/
@@ -901,6 +898,7 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
901898
if (channel->device_obj) {
902899
if (channel->chn_rescind_callback) {
903900
channel->chn_rescind_callback(channel);
901+
vmbus_rescind_cleanup(channel);
904902
return;
905903
}
906904
/*
@@ -909,6 +907,7 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
909907
*/
910908
dev = get_device(&channel->device_obj->device);
911909
if (dev) {
910+
vmbus_rescind_cleanup(channel);
912911
vmbus_device_unregister(channel->device_obj);
913912
put_device(dev);
914913
}
@@ -921,16 +920,16 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
921920
* 1. Close all sub-channels first
922921
* 2. Then close the primary channel.
923922
*/
923+
mutex_lock(&vmbus_connection.channel_mutex);
924+
vmbus_rescind_cleanup(channel);
924925
if (channel->state == CHANNEL_OPEN_STATE) {
925926
/*
926927
* The channel is currently not open;
927928
* it is safe for us to cleanup the channel.
928929
*/
929-
mutex_lock(&vmbus_connection.channel_mutex);
930-
hv_process_channel_removal(channel,
931-
channel->offermsg.child_relid);
932-
mutex_unlock(&vmbus_connection.channel_mutex);
930+
hv_process_channel_removal(rescind->child_relid);
933931
}
932+
mutex_unlock(&vmbus_connection.channel_mutex);
934933
}
935934
}
936935

drivers/hv/vmbus_drv.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -768,8 +768,7 @@ static void vmbus_device_release(struct device *device)
768768
struct vmbus_channel *channel = hv_dev->channel;
769769

770770
mutex_lock(&vmbus_connection.channel_mutex);
771-
hv_process_channel_removal(channel,
772-
channel->offermsg.child_relid);
771+
hv_process_channel_removal(channel->offermsg.child_relid);
773772
mutex_unlock(&vmbus_connection.channel_mutex);
774773
kfree(hv_dev);
775774

drivers/misc/mei/hw-me-regs.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@
127127
#define MEI_DEV_ID_BXT_M 0x1A9A /* Broxton M */
128128
#define MEI_DEV_ID_APL_I 0x5A9A /* Apollo Lake I */
129129

130+
#define MEI_DEV_ID_GLK 0x319A /* Gemini Lake */
131+
130132
#define MEI_DEV_ID_KBP 0xA2BA /* Kaby Point */
131133
#define MEI_DEV_ID_KBP_2 0xA2BB /* Kaby Point 2 */
132134

drivers/misc/mei/pci-me.c

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ static const struct pci_device_id mei_me_pci_tbl[] = {
9393
{MEI_PCI_DEVICE(MEI_DEV_ID_BXT_M, MEI_ME_PCH8_CFG)},
9494
{MEI_PCI_DEVICE(MEI_DEV_ID_APL_I, MEI_ME_PCH8_CFG)},
9595

96+
{MEI_PCI_DEVICE(MEI_DEV_ID_GLK, MEI_ME_PCH8_CFG)},
97+
9698
{MEI_PCI_DEVICE(MEI_DEV_ID_KBP, MEI_ME_PCH8_CFG)},
9799
{MEI_PCI_DEVICE(MEI_DEV_ID_KBP_2, MEI_ME_PCH8_CFG)},
98100

@@ -226,12 +228,15 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
226228
pdev->dev_flags |= PCI_DEV_FLAGS_NEEDS_RESUME;
227229

228230
/*
229-
* For not wake-able HW runtime pm framework
230-
* can't be used on pci device level.
231-
* Use domain runtime pm callbacks instead.
232-
*/
233-
if (!pci_dev_run_wake(pdev))
234-
mei_me_set_pm_domain(dev);
231+
* ME maps runtime suspend/resume to D0i states,
232+
* hence we need to go around native PCI runtime service which
233+
* eventually brings the device into D3cold/hot state,
234+
* but the mei device cannot wake up from D3 unlike from D0i3.
235+
* To get around the PCI device native runtime pm,
236+
* ME uses runtime pm domain handlers which take precedence
237+
* over the driver's pm handlers.
238+
*/
239+
mei_me_set_pm_domain(dev);
235240

236241
if (mei_pg_is_enabled(dev))
237242
pm_runtime_put_noidle(&pdev->dev);
@@ -271,8 +276,7 @@ static void mei_me_shutdown(struct pci_dev *pdev)
271276
dev_dbg(&pdev->dev, "shutdown\n");
272277
mei_stop(dev);
273278

274-
if (!pci_dev_run_wake(pdev))
275-
mei_me_unset_pm_domain(dev);
279+
mei_me_unset_pm_domain(dev);
276280

277281
mei_disable_interrupts(dev);
278282
free_irq(pdev->irq, dev);
@@ -300,8 +304,7 @@ static void mei_me_remove(struct pci_dev *pdev)
300304
dev_dbg(&pdev->dev, "stop\n");
301305
mei_stop(dev);
302306

303-
if (!pci_dev_run_wake(pdev))
304-
mei_me_unset_pm_domain(dev);
307+
mei_me_unset_pm_domain(dev);
305308

306309
mei_disable_interrupts(dev);
307310

0 commit comments

Comments
 (0)