linux-kernel - Re: [PATCH] s390: correct misuses of module_put in appldata_generic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140318143407.0df4457f@thinkpad>
Date:	Tue, 18 Mar 2014 14:34:07 +0100
From:	Gerald Schaefer <gerald.schaefer@...ibm.com>
To:	Zhouyi Zhou <zhouzhouyi@...il.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-s390@...r.kernel.org, joe@...ches.com, gang.chen@...anux.com,
	linux390@...ibm.com, heiko.carstens@...ibm.com,
	schwidefsky@...ibm.com, Zhouyi Zhou <yizhouzhou@....ac.cn>
Subject: Re: [PATCH] s390: correct misuses of module_put in
 appldata_generic_handler.

On Tue, 18 Mar 2014 09:24:55 +0800
Zhouyi Zhou <zhouzhouyi@...il.com> wrote:

> Thanks Gerald for reviewing and sorry for not elaborated it in the e-mail.
> 
> Firstly, I think you can't call module_put after fail try_module_get

There is another try_module_get() in this function before that, so I need
to call module_put() once if the second try_module_get() fails.

The first try_module_get() is only needed within the function, while the
second one is needed to prevent a module from being unloaded while its
callback is registered, so that reference is kept as long as the module
is active.

> 
> Secondly, there exists duplicate module_put on the program path (the last
> one is before return 0)

There is one module_put() for every try_module_get(). If "1" is written
to the sysctl, the function exits with one "missing" module_put(), which
is held to protect the callback. When "0" is written, this reference
is returned again with an "extra" module_put().

> 
> On Mon, Mar 17, 2014 at 9:28 PM, Gerald Schaefer
> <gerald.schaefer@...ibm.com> wrote:
> > On Sat, 15 Mar 2014 21:35:40 +0800
> > Zhouyi Zhou <zhouzhouyi@...il.com> wrote:
> >
> >> correct misuses of module_put in  appldata_generic_handler
> >
> > Sorry, I don't see any misuse, could you elaborate?
> >
> >>
> >> Signed-off-by: Zhouyi Zhou <yizhouzhou@....ac.cn>
> >> ---
> >>  arch/s390/appldata/appldata_base.c | 3 ---
> >>  1 file changed, 3 deletions(-)
> >>
> >> diff --git a/arch/s390/appldata/appldata_base.c b/arch/s390/appldata/appldata_base.c
> >> index 47c8630..683e0282 100644
> >> --- a/arch/s390/appldata/appldata_base.c
> >> +++ b/arch/s390/appldata/appldata_base.c
> >> @@ -343,7 +343,6 @@ appldata_generic_handler(struct ctl_table *ctl, int write,
> >>               // protect work queue callback
> >>               if (!try_module_get(ops->owner)) {
> >>                       mutex_unlock(&appldata_ops_mutex);
> >> -                     module_put(ops->owner);
> >>                       return -ENODEV;
> >>               }
> >>               ops->callback(ops->data);       // init record
> >> @@ -354,7 +353,6 @@ appldata_generic_handler(struct ctl_table *ctl, int write,
> >>               if (rc != 0) {
> >>                       pr_err("Starting the data collection for %s "
> >>                              "failed with rc=%d\n", ops->name, rc);
> >> -                     module_put(ops->owner);
> >>               } else
> >>                       ops->active = 1;
> >>       } else if ((buf[0] == '0') && (ops->active == 1)) {
> >> @@ -365,7 +363,6 @@ appldata_generic_handler(struct ctl_table *ctl, int write,
> >>               if (rc != 0)
> >>                       pr_err("Stopping the data collection for %s "
> >>                              "failed with rc=%d\n", ops->name, rc);
> >> -             module_put(ops->owner);
> >>       }
> >>       mutex_unlock(&appldata_ops_mutex);
> >>  out:
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-s390" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/