2022-03-31 03:42:29

by Corey Minyard

Subject: [PATCH 2/4] ipmi: Limit the number of message a user may have outstanding

From: Corey Minyard <[email protected]>

This way a rogue application can't use up a bunch of memory.

Based on work by Chen Guanqiao <[email protected]>

Cc: Chen Guanqiao <[email protected]>
Signed-off-by: Corey Minyard <[email protected]>
---
drivers/char/ipmi/ipmi_msghandler.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index de80bf4c4e4c..2a05199e8224 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -151,6 +151,12 @@ module_param(max_users, uint, 0644);
MODULE_PARM_DESC(max_users,
"The most users that may use the IPMI stack at one time.");

+/* The default maximum number of message a user may have outstanding. */
+static unsigned int max_msgs_per_user = 100;
+module_param(max_msgs_per_user, uint, 0644);
+MODULE_PARM_DESC(max_msgs_per_user,
+ "The most message a user may have outstanding.");
+
/* Call every ~1000 ms. */
#define IPMI_TIMEOUT_TIME 1000

@@ -193,6 +199,8 @@ struct ipmi_user {
/* Does this interface receive IPMI events? */
bool gets_events;

+ atomic_t nr_msgs;
+
/* Free must run in process context for RCU cleanup. */
struct work_struct remove_work;
};
@@ -934,11 +942,13 @@ static int deliver_response(struct ipmi_smi *intf, struct ipmi_recv_msg *msg)
* risk. At this moment, simply skip it in that case.
*/
ipmi_free_recv_msg(msg);
+ atomic_dec(&msg->user->nr_msgs);
} else {
int index;
struct ipmi_user *user = acquire_ipmi_user(msg->user, &index);

if (user) {
+ atomic_dec(&user->nr_msgs);
user->handler->ipmi_recv_hndl(msg, user->handler_data);
release_ipmi_user(user, index);
} else {
@@ -1256,6 +1266,7 @@ int ipmi_create_user(unsigned int if_num,
/* Note that each existing user holds a refcount to the interface. */
kref_get(&intf->refcount);

+ atomic_set(&new_user->nr_msgs, 0);
kref_init(&new_user->refcount);
new_user->handler = handler;
new_user->handler_data = handler_data;
@@ -2298,6 +2309,14 @@ static int i_ipmi_request(struct ipmi_user *user,
struct ipmi_recv_msg *recv_msg;
int rv = 0;

+ if (user) {
+ if (atomic_add_return(1, &user->nr_msgs) > max_msgs_per_user) {
+ atomic_dec(&user->nr_msgs);
+ rv = -EBUSY;
+ goto out;
+ }
+ }
+
if (supplied_recv)
recv_msg = supplied_recv;
else {
@@ -2369,6 +2388,8 @@ static int i_ipmi_request(struct ipmi_user *user,
rcu_read_unlock();

out:
+ if (rv && user)
+ atomic_dec(&user->nr_msgs);
return rv;
}

--
2.25.1

2022-03-31 04:11:23

by chenchacha

Subject: Re: [PATCH 2/4] ipmi: Limit the number of message a user may have outstanding

> @@ -2298,6 +2309,14 @@ static int i_ipmi_request(struct ipmi_user *user,
> struct ipmi_recv_msg *recv_msg;
> int rv = 0;
>
> + if (user) {
> + if (atomic_add_return(1, &user->nr_msgs) > max_msgs_per_user) {
> + atomic_dec(&user->nr_msgs);
> + rv = -EBUSY;
> + goto out;
> + }
> + }
> +
> @@ -2369,6 +2388,8 @@ static int i_ipmi_request(struct ipmi_user *user,
> rcu_read_unlock();
>
> out:
> + if (rv && user)
> + atomic_dec(&user->nr_msgs);
> return rv;
> }

If the number of msg is greater than the limit, the nr_msgs will be
decrease twice.

Should it be returned directory?

--
Chen Guanqiao

2022-03-31 04:18:12

by Corey Minyard

Subject: Re: [PATCH 2/4] ipmi: Limit the number of message a user may have outstanding

On Wed, Mar 30, 2022 at 10:44:50PM +0800, chenchacha wrote:
> > @@ -2298,6 +2309,14 @@ static int i_ipmi_request(struct ipmi_user *user,
> > struct ipmi_recv_msg *recv_msg;
> > int rv = 0;
> > + if (user) {
> > + if (atomic_add_return(1, &user->nr_msgs) > max_msgs_per_user) {
> > + atomic_dec(&user->nr_msgs);
> > + rv = -EBUSY;
> > + goto out;
> > + }
> > + }
> > +
> > @@ -2369,6 +2388,8 @@ static int i_ipmi_request(struct ipmi_user *user,
> > rcu_read_unlock();
> > out:
> > + if (rv && user)
> > + atomic_dec(&user->nr_msgs);
> > return rv;
> > }
>
> If the number of msg is greater than the limit, the nr_msgs will be decrease
> twice.
>
> Should it be returned directory?

Oh wait, yeah, I screwed that up. I added the first decrement later,
after I "noticed" it was missing. I should add a comment there.
Thanks.

-corey

>
> --
> Chen Guanqiao