[Openipmi-developer] [PATCH] ipmi: fix use after free in _ipmi_destroy_user()

2022-11-17 Thread Dan Carpenter 
The intf_free() function frees the "intf" pointer so we cannot
dereference it again on the next line.

Fixes: cbb79863fc31 ("ipmi: Don't allow device module unload when in use")
Signed-off-by: Dan Carpenter 
---
 drivers/char/ipmi/ipmi_msghandler.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c 
b/drivers/char/ipmi/ipmi_msghandler.c
index f6b8ca6df9b5..186f1fee7534 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -1330,6 +1330,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
unsigned longflags;
struct cmd_rcvr  *rcvr;
struct cmd_rcvr  *rcvrs = NULL;
+   struct module*owner;
 
if (!acquire_ipmi_user(user, )) {
/*
@@ -1392,8 +1393,9 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
kfree(rcvr);
}
 
+   owner = intf->owner;
kref_put(>refcount, intf_free);
-   module_put(intf->owner);
+   module_put(owner);
 }
 
 int ipmi_destroy_user(struct ipmi_user *user)
-- 
2.35.1



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

Re: [Openipmi-developer] [linux-next:master] BUILD REGRESSION 088b9c375534d905a4d337c78db3b3bfbb52c4a0

2022-07-08 Thread Dan Carpenter 
On Thu, Jul 07, 2022 at 07:02:58AM -0700, Guenter Roeck wrote:
> and the NULL
> dereferences in the binder driver are at the very least suspicious.

The NULL dereferences in binder are just nonsense Sparse annotations.
They don't affect runtime.

drivers/android/binder.c:1481:19-23: ERROR: from is NULL but dereferenced.
drivers/android/binder.c:2920:29-33: ERROR: target_thread is NULL but 
dereferenced.
drivers/android/binder.c:353:25-35: ERROR: node -> proc is NULL but 
dereferenced.
drivers/android/binder.c:4888:16-20: ERROR: t is NULL but dereferenced.

regards,
dan carpenter



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

Re: [Openipmi-developer] [PATCH v6 3/4] ipmi: ssif_bmc: Return -EFAULT if copy_from_user() fails

2022-03-17 Thread Dan Carpenter 
On Thu, Mar 17, 2022 at 02:45:19PM +0700, Quan Nguyen wrote:
> Added Dan as I have missed Dan's email address in the first place.
> My apologize,
> - Quan
> 
> On 11/03/2022 13:58, Wolfram Sang wrote:
> > On Thu, Mar 10, 2022 at 06:41:18PM +0700, Quan Nguyen wrote:
> > > From: Dan Carpenter 
> > > 
> > > The copy_from_user() function returns the number of bytes remaining to
> > > be copied but we should return -EFAULT here.
> > > 
> > > Fixes: 501c25b59508 ("ipmi: ssif_bmc: Add SSIF BMC driver")
> > > Signed-off-by: Dan Carpenter 
> > > Signed-off-by: Corey Minyard 
> > > Signed-off-by: Quan Nguyen 
> > 
> > It is nice that you want to keep this patch seperate to give Dan
> > credits, but I still think it should be merged into patch 1, so the
> > initial driver is as flawless as it can be. You could give Dan still
> > credits by mentioning him in the commit message IMO. Dan, would you be
> > fine with this?

Yes, that's no problem.  Thanks!

regards,
dan carpenter



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

Re: [Openipmi-developer] [PATCH] ipmi: kcs_bmc: Fix a memory leak in the error handling path of 'kcs_bmc_serio_add_device()'

2021-09-08 Thread Dan Carpenter 
On Wed, Sep 08, 2021 at 08:50:14AM +0200, Marion et Christophe JAILLET wrote:
> 
>  
> 
> > Message du 08/09/21 08:28
> > De : "Dan Carpenter" 
> > A : "Christophe JAILLET" 
> > Copie à : miny...@acm.org, zwe...@equinix.com, and...@aj.id.au, 
> > openipmi-developer@lists.sourceforge.net, linux-ker...@vger.kernel.org, 
> > kernel-janit...@vger.kernel.org
> > Objet : Re: [PATCH] ipmi: kcs_bmc: Fix a memory leak in the error handling 
> > path of 'kcs_bmc_serio_add_device()'
> > 
> > On Tue, Sep 07, 2021 at 11:06:32PM +0200, Christophe JAILLET wrote:
> > > In the unlikely event where 'devm_kzalloc()' fails and 'kzalloc()'
> > > succeeds, 'port' would be leaking.
> > > 
> > > Test each allocation separately to avoid the leak.
> > > 
> > > Fixes: 3a3d2f6a4c64 ("ipmi: kcs_bmc: Add serio adaptor")
> > > Signed-off-by: Christophe JAILLET 
> > > ---
> > > drivers/char/ipmi/kcs_bmc_serio.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/char/ipmi/kcs_bmc_serio.c 
> > > b/drivers/char/ipmi/kcs_bmc_serio.c
> > > index 7948cabde50b..7e2067628a6c 100644
> > > --- a/drivers/char/ipmi/kcs_bmc_serio.c
> > > +++ b/drivers/char/ipmi/kcs_bmc_serio.c
> > > @@ -73,10 +73,12 @@ static int kcs_bmc_serio_add_device(struct 
> > > kcs_bmc_device *kcs_bmc)
> > > struct serio *port;
> > > 
> > > priv = devm_kzalloc(kcs_bmc->dev, sizeof(*priv), GFP_KERNEL);
> > > + if (!priv)
> > > + return -ENOMEM;
> > > 
> > > /* Use kzalloc() as the allocation is cleaned up with kfree() via 
> > > serio_unregister_port() */
> > 
> > The serio_unregister_port() calls serio_destroy_port() which calls
> > put_device(>dev). But I wasn't able to track it further than
> > that to the actual kfree().
> 
> Hi Dan,
> 
> Checking this release path was not the goal of this patch.

Yeah.  I was just curious.

> It was only about the VERR unlikely memory leak.
> 
> However my understanding is:
> kcs_bmc_serio_add_device
> --> serio_register_port
> --> __serio_register_port
> --> serio_init_port
> --> serio->dev.release = serio_release_port
> 
> And in serio_release_port:
> struct serio *serio = to_serio_port(dev);
> kfree(serio);
> 
> For me, this 'serio' looks to the one allocated by 'kcs_bmc_serio_add_device'.
> I think that the comment is correct.

Thanks.  This really helps me actually.  I could just make a list of
the functions which take a container_of(dev) get a struct serio and then
free it.  Then if there is only one function that matches that, I could
assume it's what put_device() will call.

regards,
dan carpenter




___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [PATCH] ipmi: ssif_bmc: Uninitialized return in ssif_bmc_write()

2021-07-30 Thread Dan Carpenter 
I accidentally introduced a bug in my previous patch.  The "ret"
variable needs to be initialized to prevent returning uninitialized
data.

Fixes: f9714eb04364 ("ipmi: ssif_bmc: Return -EFAULT if copy_from_user() fails")
Signed-off-by: Dan Carpenter 
---
Sorry!

 drivers/char/ipmi/ssif_bmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c
index ce8cd8364a3f..acdb1d9cb5c0 100644
--- a/drivers/char/ipmi/ssif_bmc.c
+++ b/drivers/char/ipmi/ssif_bmc.c
@@ -80,7 +80,7 @@ static ssize_t ssif_bmc_write(struct file *file, const char 
__user *buf, size_t
struct ssif_bmc_ctx *ssif_bmc = to_ssif_bmc(file);
struct ssif_msg msg;
unsigned long flags;
-   ssize_t ret;
+   ssize_t ret = 0;
 
if (count > sizeof(struct ssif_msg))
return -EINVAL;
-- 
2.20.1



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [PATCH] ipmi: ssif_bmc: Return -EFAULT if copy_from_user() fails

2021-07-23 Thread Dan Carpenter 
The copy_from_user() function returns the number of bytes remaining to
be copied but we should return -EFAULT here.

Fixes: 007888f365c9 ("ipmi: ssif_bmc: Add SSIF BMC driver")
Signed-off-by: Dan Carpenter 
---
 drivers/char/ipmi/ssif_bmc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c
index b15c05622e72..ce8cd8364a3f 100644
--- a/drivers/char/ipmi/ssif_bmc.c
+++ b/drivers/char/ipmi/ssif_bmc.c
@@ -85,9 +85,8 @@ static ssize_t ssif_bmc_write(struct file *file, const char 
__user *buf, size_t
if (count > sizeof(struct ssif_msg))
return -EINVAL;
 
-   ret = copy_from_user(, buf, count);
-   if (ret)
-   return ret;
+   if (copy_from_user(, buf, count))
+   return -EFAULT;
 
if (!msg.len || count < ssif_msg_len())
return -EINVAL;
-- 
2.20.1



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

Re: [Openipmi-developer] [PATCH v3 00/11] Introduce Simple atomic counters

2020-11-10 Thread Dan Carpenter 
On Fri, Oct 16, 2020 at 03:51:25PM -0700, Kees Cook wrote:
> On Fri, Oct 16, 2020 at 12:53:13PM +0200, Peter Zijlstra wrote:
> > That's like saying: "I'm too lazy to track what I've looked at already".
> > You're basically proposing to graffiti "Kees was here -- 16/10/2020" all
> > over the kernel. Just so you can see where you still need to go.
> > 
> > It says the code was (assuming your audit was correct) good at that
> > date, but has no guarantees for any moment after that.
> 
> That kind of bit-rot marking is exactly what I would like to avoid: just
> putting a comment in is pointless. Making the expectations of the usage
> become _enforced_ is the goal. And having it enforced by the _compiler_
> is key. Just adding a meaningless attribute that a static checker
> will notice some time and hope people fix them doesn't scale either
> (just look at how many sparse warnings there are).

Most Sparse warnings are false positives.  People do actually fix the
ones which matter.

I think this patchset could be useful.  I'm working on a refcounting
check for Smatch.  I want to warn about when we forget to drop a
reference on an error path.  Right now I just assume that anything with
"error", "drop" or "->stats->" in the name is just a counter.

regards,
dan carpenter



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

Re: [Openipmi-developer] [PATCH 00/18] use semicolons rather than commas to separate statements

2020-09-29 Thread Dan Carpenter 
On Tue, Sep 29, 2020 at 02:20:00PM +0200, Ard Biesheuvel wrote:
> On Sun, 27 Sep 2020 at 21:56, Julia Lawall  wrote:
> >
> > These patches replace commas by semicolons.
> 
> 
> Why?
> 

In the best case, these commas are just uninitentional mess, like typing
an extra space character or something.  I've looked at them before and
one case I see where they are introduced is when people convert a
struct initializer to code.

-   struct foo {
-   .a = 1,
-   .b = 2,
...
+   foo.a = 1,
+   foo.b = 2,

The times where commas are used deliberately to replace curly braces are
just evil.  Either way the code is cleaner with semi-colons.

regards,
dan carpenter



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [PATCH] ipmi: msghandler: Fix a signedness bug

2020-09-18 Thread Dan Carpenter 
The type for the completion codes should be unsigned char instead of
char.  If it is declared as a normal char then the conditions in
__get_device_id() are impossible because the IPMI_DEVICE_IN_FW_UPDATE_ERR
error codes are higher than 127.

drivers/char/ipmi/ipmi_msghandler.c:2449 __get_device_id()
warn: impossible condition '(bmc->cc == 209) => ((-128)-127 == 209)'

Fixes: f8910ffa81b0 ("ipmi:msghandler: retry to get device id on an error")
Signed-off-by: Dan Carpenter 
---
 drivers/char/ipmi/ipmi_msghandler.c | 2 +-
 drivers/char/ipmi/ipmi_si_intf.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c 
b/drivers/char/ipmi/ipmi_msghandler.c
index 555c3b1e4926..8774a3b8ff95 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -319,7 +319,7 @@ struct bmc_device {
intdyn_guid_set;
struct krefusecount;
struct work_struct remove_work;
-   char   cc; /* completion code */
+   unsigned char  cc; /* completion code */
 };
 #define to_bmc_device(x) container_of((x), struct bmc_device, pdev.dev)
 
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 164f85007080..0b3dbc7e39fd 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -1344,7 +1344,7 @@ static int try_get_dev_id(struct smi_info *smi_info)
resp + 2, resp_len - 2, _info->device_id);
if (rv) {
/* record completion code */
-   char cc = *(resp + 2);
+   unsigned char cc = *(resp + 2);
 
if ((cc == IPMI_DEVICE_IN_FW_UPDATE_ERR
|| cc == IPMI_DEVICE_IN_INIT_ERR
-- 
2.28.0



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

Re: [Openipmi-developer] [PATCH] block: convert tasklets to use new tasklet_setup() API

2020-08-26 Thread Dan Carpenter 
On Wed, Aug 26, 2020 at 07:21:35AM +0530, Allen Pais wrote:
> On Thu, Aug 20, 2020 at 3:09 AM James Bottomley
>  wrote:
> >
> > On Wed, 2020-08-19 at 21:54 +0530, Allen wrote:
> > > > [...]
> > > > > > Since both threads seem to have petered out, let me suggest in
> > > > > > kernel.h:
> > > > > >
> > > > > > #define cast_out(ptr, container, member) \
> > > > > > container_of(ptr, typeof(*container), member)
> > > > > >
> > > > > > It does what you want, the argument order is the same as
> > > > > > container_of with the only difference being you name the
> > > > > > containing structure instead of having to specify its type.
> > > > >
> > > > > Not to incessantly bike shed on the naming, but I don't like
> > > > > cast_out, it's not very descriptive. And it has connotations of
> > > > > getting rid of something, which isn't really true.
> > > >
> > > > Um, I thought it was exactly descriptive: you're casting to the
> > > > outer container.  I thought about following the C++ dynamic casting
> > > > style, so out_cast(), but that seemed a bit pejorative.  What about
> > > > outer_cast()?
> > > >
> > > > > FWIW, I like the from_ part of the original naming, as it has
> > > > > some clues as to what is being done here. Why not just
> > > > > from_container()? That should immediately tell people what it
> > > > > does without having to look up the implementation, even before
> > > > > this becomes a part of the accepted coding norm.
> > > >
> > > > I'm not opposed to container_from() but it seems a little less
> > > > descriptive than outer_cast() but I don't really care.  I always
> > > > have to look up container_of() when I'm using it so this would just
> > > > be another macro of that type ...
> > > >
> > >
> > >  So far we have a few which have been suggested as replacement
> > > for from_tasklet()
> > >
> > > - out_cast() or outer_cast()
> > > - from_member().
> > > - container_from() or from_container()
> > >
> > > from_container() sounds fine, would trimming it a bit work? like
> > > from_cont().
> >
> > I'm fine with container_from().  It's the same form as container_of()
> > and I think we need urgent agreement to not stall everything else so
> > the most innocuous name is likely to get the widest acceptance.
> 
> Kees,
> 
>   Will you be  sending the newly proposed API to Linus? I have V2
> which uses container_from()
> ready to be sent out.

I liked that James swapped the first two arguments so that it matches
container_of().  Plus it's nice that when you have:

struct whatever *foo = container_from(ptr, foo, member);

Then it means that "ptr == >member".

regards,
dan carpenter



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [PATCH] ipmi: kcs: Fix aspeed_kcs_probe_of_v1()

2020-04-07 Thread Dan Carpenter 
This needs to return the newly allocated struct but instead it returns
zero which leads to an immediate Oops in the caller.

Fixes: 09f5f680707e ("ipmi: kcs: aspeed: Implement v2 bindings")
Signed-off-by: Dan Carpenter 
---
 drivers/char/ipmi/kcs_bmc_aspeed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c 
b/drivers/char/ipmi/kcs_bmc_aspeed.c
index 9422d55a0476..a140203c079b 100644
--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
@@ -271,7 +271,7 @@ static struct kcs_bmc *aspeed_kcs_probe_of_v1(struct 
platform_device *pdev)
kcs->ioreg = ast_kcs_bmc_ioregs[channel - 1];
aspeed_kcs_set_address(kcs, slave);
 
-   return 0;
+   return kcs;
 }
 
 static int aspeed_kcs_calculate_channel(const struct kcs_ioreg *regs)
-- 
2.25.1



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [PATCH] ipmi_ssif: Fix locking on probe error path

2019-08-24 Thread Dan Carpenter 
If the allocations failed then we returned with the lock held.  This
patch moves the allocations infront of the locking.

Fixes: 714acbc6cc39 ("ipmi_ssif: avoid registering duplicate ssif interface")
Signed-off-by: Dan Carpenter 
---
 drivers/char/ipmi/ipmi_ssif.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 205726926bd3..9cf2efd33f19 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -1683,7 +1683,6 @@ static int ssif_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
u8slave_addr = 0;
struct ssif_addr_info *addr_info = NULL;
 
-   mutex_lock(_infos_mutex);
resp = kmalloc(IPMI_MAX_MSG_LENGTH, GFP_KERNEL);
if (!resp)
return -ENOMEM;
@@ -1694,6 +1693,8 @@ static int ssif_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
return -ENOMEM;
}
 
+   mutex_lock(_infos_mutex);
+
if (!check_acpi(ssif_info, >dev)) {
addr_info = ssif_info_find(client->addr, client->adapter->name,
   true);
-- 
2.20.1



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [PATCH] ipmi_si: remove an unused variable in try_smi_init()

2019-03-22 Thread Dan Carpenter 
The "init_name" variable isn't used any more after commit 90b2d4f15ff7
("ipmi_si: Remove hacks for adding a dummy platform devices").

Signed-off-by: Dan Carpenter 
---
 drivers/char/ipmi/ipmi_si_intf.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index b1732882b97e..f124a2d2bb9f 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -1931,7 +1931,6 @@ static int try_smi_init(struct smi_info *new_smi)
 {
int rv = 0;
int i;
-   char *init_name = NULL;
 
pr_info("Trying %s-specified %s state machine at %s address 0x%lx, 
slave address 0x%x, irq %d\n",
ipmi_addr_src_to_str(new_smi->io.addr_source),
@@ -2073,7 +2072,6 @@ static int try_smi_init(struct smi_info *new_smi)
new_smi->io.io_cleanup = NULL;
}
 
-   kfree(init_name);
return rv;
 }
 
-- 
2.17.1



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [PATCH] ipmi_si: Potential array underflow in hotmod_handler()

2019-02-22 Thread Dan Carpenter 
The "ival" variable needs to signed so that we don't read before the
start of the str[] array.  This would only happen the user passed in a
module parameter that was just comprised of space characters.

Fixes: e80444ae4fc3 ("ipmi_si: Switch hotmod to use a platform device")
Signed-off-by: Dan Carpenter 
---
 drivers/char/ipmi/ipmi_si_hotmod.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_si_hotmod.c 
b/drivers/char/ipmi/ipmi_si_hotmod.c
index 1433055a9705..03140f6cdf6f 100644
--- a/drivers/char/ipmi/ipmi_si_hotmod.c
+++ b/drivers/char/ipmi/ipmi_si_hotmod.c
@@ -187,7 +187,8 @@ static int hotmod_handler(const char *val, const struct 
kernel_param *kp)
char *str = kstrdup(val, GFP_KERNEL), *curr, *next;
int  rv;
struct ipmi_plat_data h;
-   unsigned int len, ival;
+   unsigned int len;
+   int ival;
 
if (!str)
return -ENOMEM;
-- 
2.17.1



___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

Re: [Openipmi-developer] [PATCH] ipmi: msghandler: ensure error value in rv is being returned on error

2018-08-28 Thread Dan Carpenter 
On Tue, Aug 28, 2018 at 03:36:42PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Currently ipmi_set_my_LUN returns 0 instead of -EINVAL if the channel
> is out of range.  Fix this by returning the error return rv instead of 0.
> 
> Detected by Clang ("variable 'rv' set but not used")
> 
> Fixes: 048f7c3e352e ("ipmi: Properly release srcu locks on error conditions")
> Signed-off-by: Colin Ian King 

Somebody just sent that some hours ago.

regards,
dan carpenter


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [PATCH v2] ipmi: Fix error code in try_smi_init()

2018-03-06 Thread Dan Carpenter 
If platform_device_alloc() fails, then we should return -ENOMEM instead
of returning success.  I've also removed the "rv" initialization so that
GCC can maybe detect if we forget to set it in the future.

Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
v2: Don't initialize "rv" and fix a typo in the commit message

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index f2a294f78892..c09b279cd55e 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -2026,7 +2026,7 @@ int ipmi_si_add_smi(struct si_sm_io *io)
  */
 static int try_smi_init(struct smi_info *new_smi)
 {
-   int rv = 0;
+   int rv;
int i;
char *init_name = NULL;
 
@@ -2071,6 +2071,7 @@ static int try_smi_init(struct smi_info *new_smi)
  new_smi->intf_num);
if (!new_smi->pdev) {
pr_err(PFX "Unable to allocate platform device\n");
+   rv = -ENOMEM;
goto out_err;
}
new_smi->io.dev = _smi->pdev->dev;

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

Re: [Openipmi-developer] [PATCH] ipmi: missing error code in try_smi_init()

2018-03-06 Thread Dan Carpenter 
On Tue, Mar 06, 2018 at 11:26:19AM +0100, Julia Lawall wrote:
> 
> 
> On Tue, 6 Mar 2018, Dan Carpenter wrote:
> 
> > If platform_device_alloc() then we should return -ENOMEM instead of
> > returning success.
> 
> It looks like the word "fails" got lost here.
> 

Thanks.  Let me resend.

regards,
dan carpenter


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [PATCH] ipmi: missing error code in try_smi_init()

2018-03-06 Thread Dan Carpenter 
If platform_device_alloc() then we should return -ENOMEM instead of
returning success.

Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index f2a294f78892..ff870aa91cfe 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -2071,6 +2071,7 @@ static int try_smi_init(struct smi_info *new_smi)
  new_smi->intf_num);
if (!new_smi->pdev) {
pr_err(PFX "Unable to allocate platform device\n");
+   rv = -ENOMEM;
goto out_err;
}
new_smi->io.dev = _smi->pdev->dev;

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [bug report] ipmi: Add SMBus interface driver (SSIF)

2017-06-30 Thread Dan Carpenter 
;
   813  }
   814  break;
   815  
   816  case SSIF_GETTING_MESSAGES:
   817  if ((result < 0) || (len < 3) || (msg->rsp[2] != 0)) {
   818  /* Error getting event, probably done. */
   819  msg->done(msg);
   820  
   821  /* Take off the msg flag. */
   822  ssif_info->msg_flags &= ~RECEIVE_MSG_AVAIL;
   823  handle_flags(ssif_info, flags);
   824  } else if (msg->rsp[0] != (IPMI_NETFN_APP_REQUEST | 1) 
<< 2
   825 || msg->rsp[1] != IPMI_GET_MSG_CMD) {
   826  pr_warn(PFX "Invalid response clearing flags: 
%x %x\n",
   827  msg->rsp[0], msg->rsp[1]);
   828  msg->done(msg);
   829  
   830  /* Take off the msg flag. */
   831  ssif_info->msg_flags &= ~RECEIVE_MSG_AVAIL;
   832  handle_flags(ssif_info, flags);
   833  } else {
   834  ssif_inc_stat(ssif_info, incoming_messages);
   835  handle_flags(ssif_info, flags);
   836  deliver_recv_msg(ssif_info, msg);
   837  }
   838  break;
   839  }
   840  
   841  flags = ipmi_ssif_lock_cond(ssif_info, );
^^
Deadlock.

   842  if (SSIF_IDLE(ssif_info) && !ssif_info->stopping) {
   843  if (ssif_info->req_events)



regards,
dan carpenter

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [PATCH] ipmi_ssif: unlock on allocation failure

2017-05-04 Thread Dan Carpenter 
We should unlock and re-enable IRQs if this allocation fails.

Fixes: 259307074bfc ("ipmi: Add SMBus interface driver (SSIF) ")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>

diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 0b22a9be5029..6dd6476ea5d3 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -408,6 +408,7 @@ static void start_event_fetch(struct ssif_info *ssif_info, 
unsigned long *flags)
msg = ipmi_alloc_smi_msg();
if (!msg) {
ssif_info->ssif_state = SSIF_NORMAL;
+   ipmi_ssif_unlock_cond(ssif_info, flags);
return;
}
 
@@ -430,6 +431,7 @@ static void start_recv_msg_fetch(struct ssif_info 
*ssif_info,
msg = ipmi_alloc_smi_msg();
if (!msg) {
ssif_info->ssif_state = SSIF_NORMAL;
+   ipmi_ssif_unlock_cond(ssif_info, flags);
return;
}
 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [bug report] ipmi: shift wrap warning in STORE_SEQ_IN_MSGID()

2016-11-10 Thread Dan Carpenter 
Hi Corey,

The patch 1da177e4c3f4: "Linux-2.6.12-rc2" from Apr 16, 2005, leads
to the following static checker warning:

drivers/char/ipmi/ipmi_msghandler.c:1760 i_ipmi_request()
warn: potential shift truncation.  '0xff << 26'


drivers/char/ipmi/ipmi_msghandler.c
   157  /*
   158   * Store the information in a msgid (long) to allow us to find a
   159   * sequence table entry from the msgid.
   160   */
   161  #define STORE_SEQ_IN_MSGID(seq, seqid) (((seq&0xff)<<26) | 
(seqid&0x3ff))

The STORE_SEQ_IN_MSGID() and GET_SEQ_FROM_MSGID() don't match.

It seems clear enough that it should be (((seq & 0x3f) << 26) and that
would silence the static checker warning.  But I think also that in
GET_SEQ_FROM_MSGID() and NEXT_SEQID() we should be using 0x3ff
(with 6 f's instead of 5).

   162  
   163  #define GET_SEQ_FROM_MSGID(msgid, seq, seqid) \
   164  do {
\
   165  seq = ((msgid >> 26) & 0x3f);   
\
   166  seqid = (msgid & 0x3f); 
\
   167  } while (0)
   168  
   169  #define NEXT_SEQID(seqid) (((seqid) + 1) & 0x3f)
   170  

regards,
dan carpenter

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [patch] ipmi_ssif: silence an uninitialized variable warning

2016-04-15 Thread Dan Carpenter 
My static checker complains that "ret" could be zero here.  (Presumably
that means the hardware is badly busted).  But the result would be that
the caller assumes *resp_len is initialized when it's not and it leads
to a warning.  Let's just silence the warning.

Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>

diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 8b3be8b..512c5b6 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -1232,6 +1232,8 @@ static int do_cmd(struct i2c_client *client, int len, 
unsigned char *msg,
break;
}
 
+   if (ret == 0)
+   ret = -EINVAL;
if (ret > 0) {
/* Validate that the response is correct. */
if (ret < 3 ||

--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

[Openipmi-developer] [patch v2] ipmi: info leak in compat_ipmi_ioctl()

2013-05-31 Thread Dan Carpenter 
On x86_64 there is a 4 byte hole between -recv_type and -addr.

Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
v2: fixed the changelog a little.  Also added LKML because the
openipmi is a moderated list (and the moderator thought my email was
spam).

diff --git a/drivers/char/ipmi/ipmi_devintf.c b/drivers/char/ipmi/ipmi_devintf.c
index 9eb360f..8e306ac 100644
--- a/drivers/char/ipmi/ipmi_devintf.c
+++ b/drivers/char/ipmi/ipmi_devintf.c
@@ -810,6 +810,7 @@ static long compat_ipmi_ioctl(struct file *filep, unsigned 
int cmd,
struct ipmi_recv   __user *precv64;
struct ipmi_recv   recv64;
 
+   memset(recv64, 0, sizeof(recv64));
if (get_compat_ipmi_recv(recv64, compat_ptr(arg)))
return -EFAULT;
 

--
Get 100% visibility into Java/.NET code with AppDynamics Lite
It's a free troubleshooting tool designed for production
Get down to code-level detail for bottlenecks, with 2% overhead.
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap2
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer