Attachment #97157 for bug #153008

View | Details | Raw Unified | Return to bug 153008
Collapse All | Expand All




 Fixed locking of struct i2o_exec_wait in Executive-OSM
 Removed LCT Notify in i2o_exec_probe() which caused freeing memory and
 accessing freed memory during first enumeration of I2O devices
 Added missing locking in i2o_exec_lct_notify()
 removed put_device() of I2O controller in i2o_iop_remove() which caused
 the controller structure get freed to early
++ b/drivers/message/i2o/exec-osm.c
 Fixed access to freed memory in i2o_msg_get()
--
-- a/drivers/message/i2o/exec-osm.c

	u32 m;			/* message id */
	struct i2o_message *msg;	/* pointer to the reply message */
	struct list_head list;	/* node in global wait list */
	spinlock_t lock;	/* lock before modifying */
};

/* Work struct needed to handle LCT NOTIFY replies */

		return NULL;

	INIT_LIST_HEAD(&wait->list);
	spin_lock_init(&wait->lock);

	return wait;
};

	DECLARE_WAIT_QUEUE_HEAD(wq);
	struct i2o_exec_wait *wait;
	static u32 tcntxt = 0x80000000;
	long flags;
	int rc = 0;

	wait = i2o_exec_wait_alloc();

	wait->tcntxt = tcntxt++;
	msg->u.s.tcntxt = cpu_to_le32(wait->tcntxt);

	wait->wq = &wq;
	/*
	 * we add elements to the head, because if a entry in the list will
	 * never be removed, we have to iterate over it every time
	 */
	list_add(&wait->list, &i2o_exec_wait_list);

	/*
	 * Post the message to the controller. At some point later it will
	 * return. If we time out before it returns then complete will be zero.
	 */
	i2o_msg_post(c, msg);

	wait_event_interruptible_timeout(wq, wait->complete, timeout * HZ);
		wait->wq = &wq;
		/*
		 * we add elements add the head, because if a entry in the list
		 * will never be removed, we have to iterate over it every time
		 */
		list_add(&wait->list, &i2o_exec_wait_list);

		wait_event_interruptible_timeout(wq, wait->complete,
						 timeout * HZ);

	spin_lock_irqsave(&wait->lock, flags);
	}

	wait->wq = NULL;

	if (wait->complete)
		rc = le32_to_cpu(wait->msg->body[0]) >> 24;
	else {
		i2o_exec_wait_free(wait);
	} else {
		/*
		 * We cannot remove it now. This is important. When it does
		 * terminate (which it must do if the controller has not

		rc = -ETIMEDOUT;
	}

	spin_unlock_irqrestore(&wait->lock, flags);

	if (rc != -ETIMEDOUT) {
		i2o_flush_reply(c, wait->m);
		i2o_exec_wait_free(wait);
	}

	return rc;
};


{
	struct i2o_exec_wait *wait, *tmp;
	unsigned long flags;
	static spinlock_t lock = SPIN_LOCK_UNLOCKED;
	int rc = 1;

	/*

	 * already expired. Not much we can do about that except log it for
	 * debug purposes, increase timeout, and recompile.
	 */
	spin_lock_irqsave(&lock, flags);
	list_for_each_entry_safe(wait, tmp, &i2o_exec_wait_list, list) {
		if (wait->tcntxt == context) {
			spin_lock_irqsave(&wait->lock, flags);

			list_del(&wait->list);

			wait->m = m;
			wait->msg = msg;
			wait->complete = 1;

			if (wait->wq)

			if (wait->wq) {
				wake_up_interruptible(wait->wq);
				rc = 0;
			else
				rc = -1;

			spin_unlock_irqrestore(&wait->lock, flags);

			if (rc) {
				struct device *dev;

				dev = &c->pdev->dev;

					 c->name);
				i2o_dma_free(dev, &wait->dma);
				i2o_exec_wait_free(wait);
			} else
				wake_up_interruptible(wait->wq);

			return rc;
		}
	}

	spin_unlock_irqrestore(&lock, flags);

	osm_warn("%s: Bogus reply in POST WAIT (tr-context: %08x)!\n", c->name,
		 context);


static int i2o_exec_probe(struct device *dev)
{
	struct i2o_device *i2o_dev = to_i2o_device(dev);
	struct i2o_controller *c = i2o_dev->iop;

	i2o_event_register(i2o_dev, &i2o_exec_driver, 0, 0xffffffff);

	c->exec = i2o_dev;

	i2o_exec_lct_notify(c, c->lct->change_ind + 1);

	device_create_file(dev, &dev_attr_vendor_id);
	device_create_file(dev, &dev_attr_product_id);


	struct device *dev;
	struct i2o_message *msg;

	down(&c->lct_lock);

	dev = &c->pdev->dev;

	if (i2o_dma_realloc


	i2o_msg_post(c, msg);

	up(&c->lct_lock);

	return 0;
};

++ b/drivers/message/i2o/iop.c


	/* Ask the IOP to switch to RESET state */
	i2o_iop_reset(c);

	put_device(&c->device);
}

/**


	snprintf(poolname, sizeof(poolname), "i2o_%s_msg_inpool", c->name);
	if (i2o_pool_alloc
	    (&c->in_msg, poolname, I2O_INBOUND_MSG_FRAME_SIZE * 4 + sizeof(u32),
	     I2O_MSG_INPOOL_MIN)) {
		kfree(c);
		return ERR_PTR(-ENOMEM);
++ b/include/linux/i2o.h


	mmsg->mfa = readl(c->in_port);
	if (unlikely(mmsg->mfa >= c->in_queue.len)) {
		u32 mfa = mmsg->mfa;

		mempool_free(mmsg, c->in_msg.mempool);

		if (mfa == I2O_QUEUE_EMPTY)
			return ERR_PTR(-EBUSY);
		return ERR_PTR(-EFAULT);
	}

Return to bug 153008

Line Link Here

(-)file_not_specified_in_diff (-49 / +45 lines)
	Fixed locking of struct i2o_exec_wait in Executive-OSM
1	Removed LCT Notify in i2o_exec_probe() which caused freeing memory and
2	accessing freed memory during first enumeration of I2O devices		accessing freed memory during first enumeration of I2O devices
3	Added missing locking in i2o_exec_lct_notify()
4	removed put_device() of I2O controller in i2o_iop_remove() which caused
5	the controller structure get freed to early	1	the controller structure get freed to early
6	Fixed size of mempool in i2o_iop_alloc()	2	++ b/drivers/message/i2o/exec-osm.c
7	Fixed access to freed memory in i2o_msg_get()
8	--
9	-- a/drivers/message/i2o/exec-osm.c
Lines 55-60 struct i2o_exec_wait { Link Here
55	u32 m; /* message id */	55	u32 m; /* message id */
56	struct i2o_message msg; / pointer to the reply message */	56	struct i2o_message msg; / pointer to the reply message */
57	struct list_head list; /* node in global wait list */	57	struct list_head list; /* node in global wait list */
		58	spinlock_t lock; /* lock before modifying */
58	};	59	};
59		60
60	/* Work struct needed to handle LCT NOTIFY replies */	61	/* Work struct needed to handle LCT NOTIFY replies */
Lines 87-92 static struct i2o_exec_wait *i2o_exec_wa Link Here
87	return NULL;	88	return NULL;
88		89
89	INIT_LIST_HEAD(&wait->list);	90	INIT_LIST_HEAD(&wait->list);
		91	spin_lock_init(&wait->lock);
90		92
91	return wait;	93	return wait;
92	};	94	};
Lines 125-130 int i2o_msg_post_wait_mem(struct i2o_con Link Here
125	DECLARE_WAIT_QUEUE_HEAD(wq);	127	DECLARE_WAIT_QUEUE_HEAD(wq);
126	struct i2o_exec_wait *wait;	128	struct i2o_exec_wait *wait;
127	static u32 tcntxt = 0x80000000;	129	static u32 tcntxt = 0x80000000;
		130	long flags;
128	int rc = 0;	131	int rc = 0;
129		132
130	wait = i2o_exec_wait_alloc();	133	wait = i2o_exec_wait_alloc();
Lines 146-178 int i2o_msg_post_wait_mem(struct i2o_con Link Here
146	wait->tcntxt = tcntxt++;	149	wait->tcntxt = tcntxt++;
147	msg->u.s.tcntxt = cpu_to_le32(wait->tcntxt);	150	msg->u.s.tcntxt = cpu_to_le32(wait->tcntxt);
148		151
		152	wait->wq = &wq;
		153	/*
		154	* we add elements to the head, because if a entry in the list will
		155	* never be removed, we have to iterate over it every time
		156	*/
		157	list_add(&wait->list, &i2o_exec_wait_list);
		158
149	/*	159	/*
150	* Post the message to the controller. At some point later it will	160	* Post the message to the controller. At some point later it will
151	* return. If we time out before it returns then complete will be zero.	161	* return. If we time out before it returns then complete will be zero.
152	*/	162	*/
153	i2o_msg_post(c, msg);	163	i2o_msg_post(c, msg);
154		164
155	if (!wait->complete) {	165	wait_event_interruptible_timeout(wq, wait->complete, timeout * HZ);
156	wait->wq = &wq;
157	/*
158	* we add elements add the head, because if a entry in the list
159	* will never be removed, we have to iterate over it every time
160	*/
161	list_add(&wait->list, &i2o_exec_wait_list);
162
163	wait_event_interruptible_timeout(wq, wait->complete,
164	timeout * HZ);
165		166
166	wait->wq = NULL;	167	spin_lock_irqsave(&wait->lock, flags);
167	}
168		168
169	barrier();	169	wait->wq = NULL;
170		170
171	if (wait->complete) {	171	if (wait->complete)
172	rc = le32_to_cpu(wait->msg->body[0]) >> 24;	172	rc = le32_to_cpu(wait->msg->body[0]) >> 24;
173	i2o_flush_reply(c, wait->m);	173	else {
174	i2o_exec_wait_free(wait);
175	} else {
176	/*	174	/*
177	* We cannot remove it now. This is important. When it does	175	* We cannot remove it now. This is important. When it does
178	* terminate (which it must do if the controller has not	176	* terminate (which it must do if the controller has not
Lines 186-191 int i2o_msg_post_wait_mem(struct i2o_con Link Here
186	rc = -ETIMEDOUT;	184	rc = -ETIMEDOUT;
187	}	185	}
188		186
		187	spin_unlock_irqrestore(&wait->lock, flags);
		188
		189	if (rc != -ETIMEDOUT) {
		190	i2o_flush_reply(c, wait->m);
		191	i2o_exec_wait_free(wait);
		192	}
		193
189	return rc;	194	return rc;
190	};	195	};
191		196
Lines 213-219 static int i2o_msg_post_wait_complete(st Link Here
213	{	218	{
214	struct i2o_exec_wait wait, tmp;	219	struct i2o_exec_wait wait, tmp;
215	unsigned long flags;	220	unsigned long flags;
216	static spinlock_t lock = SPIN_LOCK_UNLOCKED;
217	int rc = 1;	221	int rc = 1;
218		222
219	/*	223	/*
Lines 223-245 static int i2o_msg_post_wait_complete(st Link Here
223	* already expired. Not much we can do about that except log it for	227	* already expired. Not much we can do about that except log it for
224	* debug purposes, increase timeout, and recompile.	228	* debug purposes, increase timeout, and recompile.
225	*/	229	*/
226	spin_lock_irqsave(&lock, flags);
227	list_for_each_entry_safe(wait, tmp, &i2o_exec_wait_list, list) {	230	list_for_each_entry_safe(wait, tmp, &i2o_exec_wait_list, list) {
228	if (wait->tcntxt == context) {	231	if (wait->tcntxt == context) {
229	list_del(&wait->list);	232	spin_lock_irqsave(&wait->lock, flags);
230		233
231	spin_unlock_irqrestore(&lock, flags);	234	list_del(&wait->list);
232		235
233	wait->m = m;	236	wait->m = m;
234	wait->msg = msg;	237	wait->msg = msg;
235	wait->complete = 1;	238	wait->complete = 1;
236		239
237	barrier();	240	if (wait->wq)
238
239	if (wait->wq) {
240	wake_up_interruptible(wait->wq);
241	rc = 0;	241	rc = 0;
242	} else {	242	else
		243	rc = -1;
		244
		245	spin_unlock_irqrestore(&wait->lock, flags);
		246
		247	if (rc) {
243	struct device *dev;	248	struct device *dev;
244		249
245	dev = &c->pdev->dev;	250	dev = &c->pdev->dev;
Lines 248-262 static int i2o_msg_post_wait_complete(st Link Here
248	c->name);	253	c->name);
249	i2o_dma_free(dev, &wait->dma);	254	i2o_dma_free(dev, &wait->dma);
250	i2o_exec_wait_free(wait);	255	i2o_exec_wait_free(wait);
251	rc = -1;	256	} else
252	}	257	wake_up_interruptible(wait->wq);
253		258
254	return rc;	259	return rc;
255	}	260	}
256	}	261	}
257		262
258	spin_unlock_irqrestore(&lock, flags);
259
260	osm_warn("%s: Bogus reply in POST WAIT (tr-context: %08x)!\n", c->name,	263	osm_warn("%s: Bogus reply in POST WAIT (tr-context: %08x)!\n", c->name,
261	context);	264	context);
262		265
Lines 322-335 static DEVICE_ATTR(product_id, S_IRUGO, Link Here
322	static int i2o_exec_probe(struct device *dev)	325	static int i2o_exec_probe(struct device *dev)
323	{	326	{
324	struct i2o_device *i2o_dev = to_i2o_device(dev);	327	struct i2o_device *i2o_dev = to_i2o_device(dev);
325	struct i2o_controller *c = i2o_dev->iop;
326		328
327	i2o_event_register(i2o_dev, &i2o_exec_driver, 0, 0xffffffff);	329	i2o_event_register(i2o_dev, &i2o_exec_driver, 0, 0xffffffff);
328		330
329	c->exec = i2o_dev;
330
331	i2o_exec_lct_notify(c, c->lct->change_ind + 1);
332
333	device_create_file(dev, &dev_attr_vendor_id);	331	device_create_file(dev, &dev_attr_vendor_id);
334	device_create_file(dev, &dev_attr_product_id);	332	device_create_file(dev, &dev_attr_product_id);
335		333
Lines 523-528 static int i2o_exec_lct_notify(struct i2 Link Here
523	struct device *dev;	521	struct device *dev;
524	struct i2o_message *msg;	522	struct i2o_message *msg;
525		523
		524	down(&c->lct_lock);
		525
526	dev = &c->pdev->dev;	526	dev = &c->pdev->dev;
527		527
528	if (i2o_dma_realloc	528	if (i2o_dma_realloc
Lines 545-550 static int i2o_exec_lct_notify(struct i2 Link Here
545		545
546	i2o_msg_post(c, msg);	546	i2o_msg_post(c, msg);
547		547
		548	up(&c->lct_lock);
		549
548	return 0;	550	return 0;
549	};	551	};
550		552
551	-- a/drivers/message/i2o/iop.c	553	++ b/drivers/message/i2o/iop.c
Lines 804-811 void i2o_iop_remove(struct i2o_controlle Link Here
804		804
805	/* Ask the IOP to switch to RESET state */	805	/* Ask the IOP to switch to RESET state */
806	i2o_iop_reset(c);	806	i2o_iop_reset(c);
807
808	put_device(&c->device);
809	}	807	}
810		808
811	/**	809	/**
Lines 1059-1065 struct i2o_controller *i2o_iop_alloc(voi Link Here
1059		1057
1060	snprintf(poolname, sizeof(poolname), "i2o_%s_msg_inpool", c->name);	1058	snprintf(poolname, sizeof(poolname), "i2o_%s_msg_inpool", c->name);
1061	if (i2o_pool_alloc	1059	if (i2o_pool_alloc
1062	(&c->in_msg, poolname, I2O_INBOUND_MSG_FRAME_SIZE * 4,	1060	(&c->in_msg, poolname, I2O_INBOUND_MSG_FRAME_SIZE * 4 + sizeof(u32),
1063	I2O_MSG_INPOOL_MIN)) {	1061	I2O_MSG_INPOOL_MIN)) {
1064	kfree(c);	1062	kfree(c);
1065	return ERR_PTR(-ENOMEM);	1063	return ERR_PTR(-ENOMEM);
1066	-- a/include/linux/i2o.h	1064	++ b/include/linux/i2o.h
Lines 1114-1121 static inline struct i2o_message *i2o_ms Link Here
1114		1114
1115	mmsg->mfa = readl(c->in_port);	1115	mmsg->mfa = readl(c->in_port);
1116	if (unlikely(mmsg->mfa >= c->in_queue.len)) {	1116	if (unlikely(mmsg->mfa >= c->in_queue.len)) {
		1117	u32 mfa = mmsg->mfa;
		1118
1117	mempool_free(mmsg, c->in_msg.mempool);	1119	mempool_free(mmsg, c->in_msg.mempool);
1118	if(mmsg->mfa == I2O_QUEUE_EMPTY)	1120
		1121	if (mfa == I2O_QUEUE_EMPTY)
1119	return ERR_PTR(-EBUSY);	1122	return ERR_PTR(-EBUSY);
1120	return ERR_PTR(-EFAULT);	1123	return ERR_PTR(-EFAULT);
1121	}	1124	}