[PATCH] staging: binder: cleanup dereference of noderef expressions
Clean up sparse warnings for cred struct dereference.
Signed-off-by: Jerry Snitselaar
---
drivers/staging/android/binder.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index fc59281..6279d82 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -1313,6 +1313,7 @@ static void binder_transaction(struct binder_proc *proc,
struct binder_transaction *in_reply_to = NULL;
struct binder_transaction_log_entry *e;
uint32_t return_error;
+ const struct cred *cred = __task_cred(proc->tsk);
e = binder_transaction_log_add(&binder_transaction_log);
e->call_type = reply ? 2 : !!(tr->flags & TF_ONE_WAY);
@@ -1452,7 +1453,7 @@ static void binder_transaction(struct binder_proc *proc,
t->from = thread;
else
t->from = NULL;
- t->sender_euid = proc->tsk->cred->euid;
+ t->sender_euid = cred->euid;
t->to_proc = target_proc;
t->to_thread = target_thread;
t->code = tr->code;
@@ -2574,6 +2575,7 @@ static long binder_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
struct binder_thread *thread;
unsigned int size = _IOC_SIZE(cmd);
void __user *ubuf = (void __user *)arg;
+ const struct cred *cred = current_cred();
/*pr_info("binder_ioctl: %d:%d %x %lx\n", proc->pid, current->pid, cmd,
arg);*/
@@ -2658,15 +2660,15 @@ static long binder_ioctl(struct file *filp, unsigned
int cmd, unsigned long arg)
goto err;
}
if (uid_valid(binder_context_mgr_uid)) {
- if (!uid_eq(binder_context_mgr_uid,
current->cred->euid)) {
+ if (!uid_eq(binder_context_mgr_uid, cred->euid)) {
pr_err("BINDER_SET_CONTEXT_MGR bad uid %d !=
%d\n",
- from_kuid(&init_user_ns,
current->cred->euid),
+ from_kuid(&init_user_ns, cred->euid),
from_kuid(&init_user_ns,
binder_context_mgr_uid));
ret = -EPERM;
goto err;
}
} else
- binder_context_mgr_uid = current->cred->euid;
+ binder_context_mgr_uid = cred->euid;
binder_context_mgr_node = binder_new_node(proc, 0, 0);
if (binder_context_mgr_node == NULL) {
ret = -ENOMEM;
--
2.0.0.rc0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: fix coding style issues in comedi_fops.c
This is a patch to fix coding style
warnings found by checkpatch.pl tool
Signed-off-by: Raghavendra Chandra Ganiga
---
drivers/staging/comedi/comedi_fops.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 70545e6..b24bf9b 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -668,6 +668,7 @@ static int do_devconfig_ioctl(struct comedi_device *dev,
return -EBUSY;
if (dev->attached) {
struct module *driver_module = dev->driver->module;
+
comedi_device_detach(dev);
module_put(driver_module);
}
@@ -2653,6 +2654,7 @@ static int __init comedi_init(void)
/* create devices files for legacy/manual use */
for (i = 0; i < comedi_num_legacy_minors; i++) {
struct comedi_device *dev;
+
dev = comedi_alloc_board_minor(NULL);
if (IS_ERR(dev)) {
comedi_cleanup_board_minors();
--
1.7.10.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: fix potential leak in rtw_set_key()
Fix a potential leak in the error path of rtw_set_key(). In case the requested
algorithm is not supported by the driver, the function returns without
enqueuing or freeing the already allocated command and parameter structs. Use
a centralized exit path and make sure that all memory is freed correctly.
Detected by Coverity - CID 1077716, 1077717.
Signed-off-by: Christian Engelmayer
---
Compile tested and applies against branch staging-next of tree
git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
---
drivers/staging/rtl8188eu/core/rtw_mlme.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index 769d4dd..275ae7b 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -1729,13 +1729,12 @@ int rtw_set_key(struct adapter *adapter, struct
security_priv *psecuritypriv, in
pcmd = (struct cmd_obj *)rtw_zmalloc(sizeof(struct cmd_obj));
if (pcmd == NULL) {
res = _FAIL; /* try again */
- goto exit;
+ goto fail_noobj;
}
psetkeyparm = (struct setkey_parm *)rtw_zmalloc(sizeof(struct
setkey_parm));
if (psetkeyparm == NULL) {
- kfree(pcmd);
res = _FAIL;
- goto exit;
+ goto fail_noparm;
}
_rtw_memset(psetkeyparm, 0, sizeof(struct setkey_parm));
@@ -1784,7 +1783,7 @@ int rtw_set_key(struct adapter *adapter, struct
security_priv *psecuritypriv, in
("\n rtw_set_key:psecuritypriv->dot11PrivacyAlgrthm=%x
(must be 1 or 2 or 4 or 5)\n",
psecuritypriv->dot11PrivacyAlgrthm));
res = _FAIL;
- goto exit;
+ goto fail;
}
pcmd->cmdcode = _SetKey_CMD_;
pcmd->parmbuf = (u8 *)psetkeyparm;
@@ -1793,7 +1792,13 @@ int rtw_set_key(struct adapter *adapter, struct
security_priv *psecuritypriv, in
pcmd->rspsz = 0;
_rtw_init_listhead(&pcmd->list);
res = rtw_enqueue_cmd(pcmdpriv, pcmd);
-exit:
+ return res;
+
+fail:
+ kfree(psetkeyparm);
+fail_noparm:
+ kfree(pcmd);
+fail_noobj:
return res;
}
--
1.9.1
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: fix coding style issues in comedi_fops.c
On 2014/04/30 06:34 PM, Raghavendra Ganiga wrote:
> This is a patch to fix coding style
> warnings found by the checkpatch.pl tool
>
> Signed-off-by: Raghavendra Chandra Ganiga
> ---
> drivers/staging/comedi/comedi_fops.c |6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/comedi/comedi_fops.c
> b/drivers/staging/comedi/comedi_fops.c
> index acc8019..a62d639 100644
> --- a/drivers/staging/comedi/comedi_fops.c
> +++ b/drivers/staging/comedi/comedi_fops.c
> @@ -1435,13 +1435,15 @@ static int __comedi_get_user_cmd(struct comedi_device
> *dev,
> s = &dev->subdevices[cmd->subdev];
>
> if (s->type == COMEDI_SUBD_UNUSED) {
> - dev_dbg(dev->class_dev, "%d not valid subdevice\n",
> cmd->subdev);
> + dev_dbg(dev->class_dev, "%d not valid subdevice\n",
> + cmd->subdev);
> return -EIO;
> }
>
> if (!s->do_cmd || !s->do_cmdtest || !s->async) {
> dev_dbg(dev->class_dev,
> - "subdevice %d does not support commands\n",
> cmd->subdev);
> + "subdevice %d does not support commands\n",
> + cmd->subdev);
> return -EIO;
> }
(I think this is "PATCH v2".)
Reviewed-by: Ian Abbott
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: fix coding style issues in comedi_fops.c
On 2014/05/01 09:23 AM, Raghavendra Ganiga wrote:
> This is a patch to fix coding style
> warnings found by checkpatch.pl tool
>
> Signed-off-by: Raghavendra Chandra Ganiga
> ---
> drivers/staging/comedi/comedi_fops.c |2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/comedi/comedi_fops.c
> b/drivers/staging/comedi/comedi_fops.c
> index 70545e6..b24bf9b 100644
> --- a/drivers/staging/comedi/comedi_fops.c
> +++ b/drivers/staging/comedi/comedi_fops.c
> @@ -668,6 +668,7 @@ static int do_devconfig_ioctl(struct comedi_device *dev,
> return -EBUSY;
> if (dev->attached) {
> struct module *driver_module = dev->driver->module;
> +
> comedi_device_detach(dev);
> module_put(driver_module);
> }
> @@ -2653,6 +2654,7 @@ static int __init comedi_init(void)
> /* create devices files for legacy/manual use */
> for (i = 0; i < comedi_num_legacy_minors; i++) {
> struct comedi_device *dev;
> +
> dev = comedi_alloc_board_minor(NULL);
> if (IS_ERR(dev)) {
> comedi_cleanup_board_minors();
>
Reviewed-by: Ian Abbott
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8188eu: fix potential leak in rtw_set_key()
On Thu, May 01, 2014 at 10:38:11AM +0200, Christian Engelmayer wrote:
> Fix a potential leak in the error path of rtw_set_key(). In case the requested
> algorithm is not supported by the driver, the function returns without
> enqueuing or freeing the already allocated command and parameter structs. Use
> a centralized exit path and make sure that all memory is freed correctly.
> Detected by Coverity - CID 1077716, 1077717.
>
> Signed-off-by: Christian Engelmayer
> ---
> Compile tested and applies against branch staging-next of tree
> git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
> ---
> drivers/staging/rtl8188eu/core/rtw_mlme.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> index 769d4dd..275ae7b 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> @@ -1729,13 +1729,12 @@ int rtw_set_key(struct adapter *adapter, struct
> security_priv *psecuritypriv, in
> pcmd = (struct cmd_obj *)rtw_zmalloc(sizeof(struct cmd_obj));
> if (pcmd == NULL) {
> res = _FAIL; /* try again */
> - goto exit;
> + goto fail_noobj;
Just return here. The do nothing goto is misleading because you assume
it is a there for a purpose but then when you read all the way to the
end, it doesn't do anything useful. It doesn't do anything at all.
The label statements should be based on the thing which is labeled and
not the goto locations. In this case it should be something like
"err_free_cmd" and "err_free_keyparm". The "fail" label is too generic
so it's not a good name.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/32] staging: comedi: continue async command cleanup
On 2014-04-30 17:46, Hartley Sweeten wrote: On Wednesday, April 30, 2014 2:13 AM, Ian Abbott wrote: As a side node, I wonder if it's worth stripping out those `| I8254_BINARY` bits as it's basically 'OR'ing with zero anyway. I like the I8254_BINARY, it documents the mode that the counter is used in. But, if you want to strip them out... Fair enough. BTW, I noticed that the i8254_set_mode() helpers have a slight "bug". if (mode > (I8254_MODE5 | I8254_BINARY)) return -1; This test will not allow the mode (I8254_MODE5 | I8254_BCD). Nothing uses it right now, and it's a bit silly to use BCD counting anyway. But... It does get exposed to user-space by a few drivers that expose the 8254 as a counter subdevice. The "adv_pci_dio" and "das08" drivers expose it when handling the INSN_CONFIG_SET_COUNTER_MODE instruction (also, "me4000" driver handling it as a GPCT_SET_OPERATION instruction whatever that is - it should probably be deprecated in favour of INSN_CONFIG_SET_COUNTER_MODE). Also, the "amplc_dio200_common" module has the same bug in its handling of INSN_CONFIG_SET_COUNTER_MODE but uses its own function to set the mode instead of calling i8254_set_mode(). I'll post some patches to fix it later. Probably not worth backporting them to "stable". -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: rtl8188eu: fix potential leak in rtw_set_key()
Fix a potential leak in the error path of rtw_set_key(). In case the requested
algorithm is not supported by the driver, the function returns without
enqueuing or freeing the already allocated command and parameter structs. Use
a centralized exit path and make sure that all memory is freed correctly.
Detected by Coverity - CID 1077716, 1077717.
Signed-off-by: Christian Engelmayer
---
v2: Added changes requested by Dan Carpenter:
* Just return directly where no cleanup is needed.
* Prefer naming labels by the labeled action rather than the goto location.
Compile tested and applies against branch staging-next of tree
git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
---
drivers/staging/rtl8188eu/core/rtw_mlme.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index 769d4dd..155282e 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -1727,15 +1727,13 @@ int rtw_set_key(struct adapter *adapter, struct
security_priv *psecuritypriv, in
int res = _SUCCESS;
pcmd = (struct cmd_obj *)rtw_zmalloc(sizeof(struct cmd_obj));
- if (pcmd == NULL) {
- res = _FAIL; /* try again */
- goto exit;
- }
+ if (pcmd == NULL)
+ return _FAIL; /* try again */
+
psetkeyparm = (struct setkey_parm *)rtw_zmalloc(sizeof(struct
setkey_parm));
if (psetkeyparm == NULL) {
- kfree(pcmd);
res = _FAIL;
- goto exit;
+ goto err_free_cmd;
}
_rtw_memset(psetkeyparm, 0, sizeof(struct setkey_parm));
@@ -1784,7 +1782,7 @@ int rtw_set_key(struct adapter *adapter, struct
security_priv *psecuritypriv, in
("\n rtw_set_key:psecuritypriv->dot11PrivacyAlgrthm=%x
(must be 1 or 2 or 4 or 5)\n",
psecuritypriv->dot11PrivacyAlgrthm));
res = _FAIL;
- goto exit;
+ goto err_free_parm;
}
pcmd->cmdcode = _SetKey_CMD_;
pcmd->parmbuf = (u8 *)psetkeyparm;
@@ -1793,7 +1791,12 @@ int rtw_set_key(struct adapter *adapter, struct
security_priv *psecuritypriv, in
pcmd->rspsz = 0;
_rtw_init_listhead(&pcmd->list);
res = rtw_enqueue_cmd(pcmdpriv, pcmd);
-exit:
+ return res;
+
+err_free_parm:
+ kfree(psetkeyparm);
+err_free_cmd:
+ kfree(pcmd);
return res;
}
--
1.9.1
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8712: fix potential leak in r871x_wx_set_enc_ext()
Fix a potential leak in the error path of r871x_wx_set_enc_ext(). In case the
requested algorithm is not supported by the driver, the function returns
without freeing the already allocated 'param' struct. Move the input
verification to the beginning of the function so that the direct return is
safe. Detected by Coverity - CID 144373.
Signed-off-by: Christian Engelmayer
---
Compile tested and applies against branch staging-next of tree
git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
---
drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 23d539d..1eca992 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -1801,13 +1801,6 @@ static int r871x_wx_set_enc_ext(struct net_device *dev,
u32 param_len;
int ret = 0;
- param_len = sizeof(struct ieee_param) + pext->key_len;
- param = (struct ieee_param *)_malloc(param_len);
- if (param == NULL)
- return -ENOMEM;
- memset(param, 0, param_len);
- param->cmd = IEEE_CMD_SET_ENCRYPTION;
- memset(param->sta_addr, 0xff, ETH_ALEN);
switch (pext->alg) {
case IW_ENCODE_ALG_NONE:
alg_name = "none";
@@ -1824,6 +1817,15 @@ static int r871x_wx_set_enc_ext(struct net_device *dev,
default:
return -EINVAL;
}
+
+ param_len = sizeof(struct ieee_param) + pext->key_len;
+ param = (struct ieee_param *)_malloc(param_len);
+ if (param == NULL)
+ return -ENOMEM;
+ memset(param, 0, param_len);
+ param->cmd = IEEE_CMD_SET_ENCRYPTION;
+ memset(param->sta_addr, 0xff, ETH_ALEN);
+
strncpy((char *)param->u.crypt.alg, alg_name, IEEE_CRYPT_ALG_NAME_LEN);
if (pext->ext_flags & IW_ENCODE_EXT_GROUP_KEY)
param->u.crypt.set_tx = 0;
--
1.9.1
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: rtl8188eu: fix potential leak in rtw_set_key()
On Thu, May 01, 2014 at 12:30:57PM +0200, Christian Engelmayer wrote: > Fix a potential leak in the error path of rtw_set_key(). In case the requested > algorithm is not supported by the driver, the function returns without > enqueuing or freeing the already allocated command and parameter structs. Use > a centralized exit path and make sure that all memory is freed correctly. > Detected by Coverity - CID 1077716, 1077717. > > Signed-off-by: Christian Engelmayer > --- > v2: Added changes requested by Dan Carpenter: > >* Just return directly where no cleanup is needed. >* Prefer naming labels by the labeled action rather than the goto location. > > Compile tested and applies against branch staging-next of tree > git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git Looks good. Reviewed-by: Dan Carpenter regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723au: fix potential leak in update_bcn_wps_ie()
Fix a potential leak in the error path of function update_bcn_wps_ie(). Make sure that allocated memory for 'pbackup_remainder_ie' is freed upon return. Detected by Coverity - CID 1077718. Signed-off-by: Christian Engelmayer --- Compile tested and applies against branch staging-next of tree git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git --- drivers/staging/rtl8723au/core/rtw_ap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/rtl8723au/core/rtw_ap.c b/drivers/staging/rtl8723au/core/rtw_ap.c index 9b31412..f2c78a7 100644 --- a/drivers/staging/rtl8723au/core/rtw_ap.c +++ b/drivers/staging/rtl8723au/core/rtw_ap.c @@ -1276,7 +1276,7 @@ static void update_bcn_wps_ie(struct rtw_adapter *padapter) pwps_ie_src = pmlmepriv->wps_beacon_ie; if (pwps_ie_src == NULL) - return; + goto exit; wps_ielen = (uint)pwps_ie_src[1];/* to get ie data len */ if ((wps_offset+wps_ielen+2+remainder_ielen)<= MAX_IE_SZ) @@ -1291,8 +1291,8 @@ static void update_bcn_wps_ie(struct rtw_adapter *padapter) pnetwork->IELength = wps_offset + (wps_ielen+2) + remainder_ielen; } - if (pbackup_remainder_ie) - kfree(pbackup_remainder_ie); +exit: + kfree(pbackup_remainder_ie); } static void update_bcn_p2p_ie(struct rtw_adapter *padapter) -- 1.9.1 signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: fix potential leak in update_bcn_wps_ie()
Fix a potential leak in the error path of function update_bcn_wps_ie().
Make sure that allocated memory for 'pbackup_remainder_ie' is freed
upon return. Detected by Coverity - CID 1077718.
Signed-off-by: Christian Engelmayer
---
Compile tested and applies against branch staging-next of tree
git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
---
drivers/staging/rtl8188eu/core/rtw_ap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c
b/drivers/staging/rtl8188eu/core/rtw_ap.c
index ff74d0d..6268f44 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -1325,7 +1325,7 @@ static void update_bcn_wps_ie(struct adapter *padapter)
pwps_ie_src = pmlmepriv->wps_beacon_ie;
if (pwps_ie_src == NULL)
- return;
+ goto exit;
wps_ielen = (uint)pwps_ie_src[1];/* to get ie data len */
if ((wps_offset+wps_ielen+2+remainder_ielen) <= MAX_IE_SZ) {
@@ -1339,6 +1339,7 @@ static void update_bcn_wps_ie(struct adapter *padapter)
pnetwork->IELength = wps_offset + (wps_ielen+2) +
remainder_ielen;
}
+exit:
kfree(pbackup_remainder_ie);
}
--
1.9.1
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: fix potential leak in update_bcn_wps_ie()
On Thu, May 01, 2014 at 01:57:27PM +0200, Christian Engelmayer wrote:
> Fix a potential leak in the error path of function update_bcn_wps_ie().
> Make sure that allocated memory for 'pbackup_remainder_ie' is freed
> upon return. Detected by Coverity - CID 1077718.
>
if (remainder_ielen > 0) {
pbackup_remainder_ie = kmalloc(remainder_ielen, GFP_ATOMIC);
if (pbackup_remainder_ie)
memcpy(pbackup_remainder_ie, premainder_ie,
remainder_ielen);
}
pwps_ie_src = pmlmepriv->wps_beacon_ie;
if (pwps_ie_src == NULL)
return;
Maybe just check pwps_ie_src earlier?
--
Mateusz Guzik
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 00/32] staging: comedi: continue async command cleanup
On Thursday, May 01, 2014 2:46 AM, Ian Abbott wrote: > On 2014-04-30 17:46, Hartley Sweeten wrote: >> On Wednesday, April 30, 2014 2:13 AM, Ian Abbott wrote: >>> As a side node, I wonder if it's worth stripping out those `| >>> I8254_BINARY` bits as it's basically 'OR'ing with zero anyway. >> >> I like the I8254_BINARY, it documents the mode that the counter >> is used in. But, if you want to strip them out... > > Fair enough. > >> BTW, I noticed that the i8254_set_mode() helpers have a slight "bug". >> >> if (mode > (I8254_MODE5 | I8254_BINARY)) >> return -1; >> >> This test will not allow the mode (I8254_MODE5 | I8254_BCD). Nothing >> uses it right now, and it's a bit silly to use BCD counting anyway. But... > > It does get exposed to user-space by a few drivers that expose the 8254 > as a counter subdevice. The "adv_pci_dio" and "das08" drivers expose it > when handling the INSN_CONFIG_SET_COUNTER_MODE instruction (also, > "me4000" driver handling it as a GPCT_SET_OPERATION instruction whatever > that is - it should probably be deprecated in favour of > INSN_CONFIG_SET_COUNTER_MODE). Also, the "amplc_dio200_common" module > has the same bug in its handling of INSN_CONFIG_SET_COUNTER_MODE but > uses its own function to set the mode instead of calling i8254_set_mode(). > > I'll post some patches to fix it later. Probably not worth backporting > them to "stable". Great! BTW, what's with all the NI_GPCT_* stuff in the main comedi.h header? These all seem pretty driver specific to ni_tio.c and ni_tiocmd.c. I don't understand why they are exposed in the user space header. Thanks, Hartley ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: comedi: amplc_dio200_common: correct bound on counter mode
For the mode configured by the `INSN_CONFIG_SET_COUNTER_MODE` comedi
instruction for the counter subdevice channels supported by this module,
the upper bound should be `I8254_MODE5 | I8254_BCD` ((5 << 1) | 1)
rather than `I8254_MODE5 | I8254_BINARY` ((5 << 1) | 0). Fix it.
Reported-by: Hartley Sweeten
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/amplc_dio200_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/amplc_dio200_common.c
b/drivers/staging/comedi/drivers/amplc_dio200_common.c
index 4ac3208..ee18537 100644
--- a/drivers/staging/comedi/drivers/amplc_dio200_common.c
+++ b/drivers/staging/comedi/drivers/amplc_dio200_common.c
@@ -820,7 +820,7 @@ dio200_subdev_8254_config(struct comedi_device *dev, struct
comedi_subdevice *s,
spin_lock_irqsave(&subpriv->spinlock, flags);
switch (data[0]) {
case INSN_CONFIG_SET_COUNTER_MODE:
- if (data[1] > (I8254_MODE5 | I8254_BINARY))
+ if (data[1] > (I8254_MODE5 | I8254_BCD))
ret = -EINVAL;
else
dio200_subdev_8254_set_mode(dev, s, chan, data[1]);
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: comedi: drivers: correct mode check for i8254_set_mode()
The upper bound check on the `mode` parameter of `i8254_set_mode()` and
`i8254_mm_set_mode()` is incorrect. The `mode` parameter value consists
of a mode number in the range 0 to 5 in bits 3..1 {represented by the
constants `I8254_MODE0` (0 << 1) through to `I8254_MODE5` (2 << 1)} ORed
with a BCD/binary flag in bit 0 {represented by the constants
`I8254_BINARY` (0) and `I8254_BCD` (1)}. The maximum allowed value
ought to be `I8254_MODE5 | I8254_BCD` ((5 << 1) | 1), but is currently
`I8254_MODE5 | I8254_BINARY` ((5 << 1) | 0). Fix it.
None of the comedi drivers use `I8254_BCD` but some of the low-level
drivers allow user-space to configure the counter mode, so all legal
values ought to be allowed. However, it's pretty unlikely anyone would
want to set the counters to count in BCD (binary coded decimal) so the
bug is not that significant.
Reported-by: Hartley Sweeten
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/8253.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/8253.h
b/drivers/staging/comedi/drivers/8253.h
index e3d737c..a66563a 100644
--- a/drivers/staging/comedi/drivers/8253.h
+++ b/drivers/staging/comedi/drivers/8253.h
@@ -386,7 +386,7 @@ static inline int i8254_set_mode(unsigned long base_address,
if (counter_number > 2)
return -1;
- if (mode > (I8254_MODE5 | I8254_BINARY))
+ if (mode > (I8254_MODE5 | I8254_BCD))
return -1;
byte = counter_number << 6;
@@ -406,7 +406,7 @@ static inline int i8254_mm_set_mode(void __iomem
*base_address,
if (counter_number > 2)
return -1;
- if (mode > (I8254_MODE5 | I8254_BINARY))
+ if (mode > (I8254_MODE5 | I8254_BCD))
return -1;
byte = counter_number << 6;
--
1.9.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/2] staging: comedi: correct upper bound check for 8254 mode
Hartley spotted a bug in the upper bound check for the counter 'mode' value in `i8254_set_mode()` (and the similar `i8254_mm_set_mode()` for memory-mapped registers). The same incorrect upper bound check was used in the "amplc_dio200_common" module which uses its own function to set the counter mode. 1) staging: comedi: drivers: correct mode check for i8254_set_mode() 2) staging: comedi: amplc_dio200_common: correct bound on counter mode drivers/staging/comedi/drivers/8253.h| 4 ++-- drivers/staging/comedi/drivers/amplc_dio200_common.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 0/2] staging: comedi: correct upper bound check for 8254 mode
On Thursday, May 01, 2014 9:38 AM, Ian Abbott wrote: > Hartley spotted a bug in the upper bound check for the counter 'mode' > value in `i8254_set_mode()` (and the similar `i8254_mm_set_mode()` for > memory-mapped registers). The same incorrect upper bound check was used > in the "amplc_dio200_common" module which uses its own function to set > the counter mode. > > 1) staging: comedi: drivers: correct mode check for i8254_set_mode() > 2) staging: comedi: amplc_dio200_common: correct bound on counter mode > > drivers/staging/comedi/drivers/8253.h| 4 ++-- > drivers/staging/comedi/drivers/amplc_dio200_common.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) Reviewed-by: H Hartley Sweeten ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/32] staging: comedi: continue async command cleanup
On 2014-05-01 17:31, Hartley Sweeten wrote: BTW, what's with all the NI_GPCT_* stuff in the main comedi.h header? These all seem pretty driver specific to ni_tio.c and ni_tiocmd.c. I don't understand why they are exposed in the user space header. Basically for the same reason the 8254 mode values are exposed, so user-space can use the constants to construct the values for configuration instructions. -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: fix potential leak in update_bcn_wps_ie()
On Thu, 1 May 2014 14:22:17 +0200, Mateusz Guzik wrote:
> On Thu, May 01, 2014 at 01:57:27PM +0200, Christian Engelmayer wrote:
> > Fix a potential leak in the error path of function update_bcn_wps_ie().
> > Make sure that allocated memory for 'pbackup_remainder_ie' is freed
> > upon return. Detected by Coverity - CID 1077718.
> >
>
> if (remainder_ielen > 0) {
> pbackup_remainder_ie = kmalloc(remainder_ielen, GFP_ATOMIC);
> if (pbackup_remainder_ie)
> memcpy(pbackup_remainder_ie, premainder_ie,
>remainder_ielen);
> }
>
> pwps_ie_src = pmlmepriv->wps_beacon_ie;
> if (pwps_ie_src == NULL)
> return;
>
>
> Maybe just check pwps_ie_src earlier?
>
You are right, I see no reason why this cannot be done early in the function.
diff --git a/drivers/staging/rtl8723au/core/rtw_ap.c
b/drivers/staging/rtl8723au/core/rtw_ap.c
index 9b31412..da028c535 100644
--- a/drivers/staging/rtl8723au/core/rtw_ap.c
+++ b/drivers/staging/rtl8723au/core/rtw_ap.c
@@ -1256,6 +1256,10 @@ static void update_bcn_wps_ie(struct rtw_adapter
*padapter)
DBG_8723A("%s\n", __func__);
+ pwps_ie_src = pmlmepriv->wps_beacon_ie;
+ if (pwps_ie_src == NULL)
+ return;
+
pwps_ie = rtw_get_wps_ie23a(ie+_FIXED_IE_LENGTH_,
ielen-_FIXED_IE_LENGTH_, NULL, &wps_ielen);
if (pwps_ie == NULL || wps_ielen == 0)
@@ -1274,10 +1278,6 @@ static void update_bcn_wps_ie(struct rtw_adapter
*padapter)
remainder_ielen);
}
- pwps_ie_src = pmlmepriv->wps_beacon_ie;
- if (pwps_ie_src == NULL)
- return;
-
wps_ielen = (uint)pwps_ie_src[1];/* to get ie data len */
if ((wps_offset+wps_ielen+2+remainder_ielen)<= MAX_IE_SZ)
{
Regards,
Christian
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: fix potential leak in update_bcn_wps_ie()
Christian Engelmayer writes:
> On Thu, 1 May 2014 14:22:17 +0200, Mateusz Guzik wrote:
>> On Thu, May 01, 2014 at 01:57:27PM +0200, Christian Engelmayer wrote:
>> > Fix a potential leak in the error path of function update_bcn_wps_ie().
>> > Make sure that allocated memory for 'pbackup_remainder_ie' is freed
>> > upon return. Detected by Coverity - CID 1077718.
>> >
>>
>> if (remainder_ielen > 0) {
>> pbackup_remainder_ie = kmalloc(remainder_ielen, GFP_ATOMIC);
>> if (pbackup_remainder_ie)
>> memcpy(pbackup_remainder_ie, premainder_ie,
>>remainder_ielen);
>> }
>>
>> pwps_ie_src = pmlmepriv->wps_beacon_ie;
>> if (pwps_ie_src == NULL)
>> return;
>>
>>
>> Maybe just check pwps_ie_src earlier?
>>
>
> You are right, I see no reason why this cannot be done early in the function.
Looks good to me - if you send me a patch with a commit message and a
Signed-off-by, I'll add it to the rtl8723au driver tree and push it to
Greg with my next set of changes.
Cheers,
Jes
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: rtl8188eu: fix potential leak in update_bcn_wps_ie()
Fix a potential leak in the error path of function update_bcn_wps_ie().
Move the affected input verification to the beginning of the function so
that it may return directly without leaking already allocated memory.
Detected by Coverity - CID 1077718.
Signed-off-by: Christian Engelmayer
---
v2: Added change suggested by Mateusz Guzik for the rtl8723au variant:
Move the check before allocating the memory instead of freeing the
resource afterwards in the error path.
Compile tested and applies against branch staging-next of tree
git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
---
drivers/staging/rtl8188eu/core/rtw_ap.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c
b/drivers/staging/rtl8188eu/core/rtw_ap.c
index ff74d0d..85fda61 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -1306,6 +1306,10 @@ static void update_bcn_wps_ie(struct adapter *padapter)
DBG_88E("%s\n", __func__);
+ pwps_ie_src = pmlmepriv->wps_beacon_ie;
+ if (pwps_ie_src == NULL)
+ return;
+
pwps_ie = rtw_get_wps_ie(ie+_FIXED_IE_LENGTH_, ielen-_FIXED_IE_LENGTH_,
NULL, &wps_ielen);
if (pwps_ie == NULL || wps_ielen == 0)
@@ -1323,10 +1327,6 @@ static void update_bcn_wps_ie(struct adapter *padapter)
memcpy(pbackup_remainder_ie, premainder_ie,
remainder_ielen);
}
- pwps_ie_src = pmlmepriv->wps_beacon_ie;
- if (pwps_ie_src == NULL)
- return;
-
wps_ielen = (uint)pwps_ie_src[1];/* to get ie data len */
if ((wps_offset+wps_ielen+2+remainder_ielen) <= MAX_IE_SZ) {
memcpy(pwps_ie, pwps_ie_src, wps_ielen+2);
--
1.9.1
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8712: fix potential leaks in r8712_set_key()
Fix potential leaks in the error paths of r8712_set_key(). In case the
algorithm specific checks fail, the function returns without enqueuing
or freeing the already allocated command and parameter structs. Use a
centralized exit path and make sure that all memory is freed correctly.
Detected by Coverity - CID 144370, 144371.
Signed-off-by: Christian Engelmayer
---
Compile tested and applies against branch staging-next of tree
git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
---
drivers/staging/rtl8712/rtl871x_mlme.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c
b/drivers/staging/rtl8712/rtl871x_mlme.c
index 3ea99ae..23fd8c1 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -1243,14 +1243,15 @@ sint r8712_set_key(struct _adapter *adapter,
struct cmd_obj *pcmd;
struct setkey_parm *psetkeyparm;
u8 keylen;
+ sint ret = _SUCCESS;
pcmd = (struct cmd_obj *)_malloc(sizeof(struct cmd_obj));
if (pcmd == NULL)
return _FAIL;
psetkeyparm = (struct setkey_parm *)_malloc(sizeof(struct setkey_parm));
if (psetkeyparm == NULL) {
- kfree((unsigned char *)pcmd);
- return _FAIL;
+ ret = _FAIL;
+ goto err_free_cmd;
}
memset(psetkeyparm, 0, sizeof(struct setkey_parm));
if (psecuritypriv->AuthAlgrthm == 2) { /* 802.1X */
@@ -1274,23 +1275,28 @@ sint r8712_set_key(struct _adapter *adapter,
psecuritypriv->DefKey[keyid].skey, keylen);
break;
case _TKIP_:
- if (keyid < 1 || keyid > 2)
- return _FAIL;
+ if (keyid < 1 || keyid > 2) {
+ ret = _FAIL;
+ goto err_free_parm;
+ }
keylen = 16;
memcpy(psetkeyparm->key,
&psecuritypriv->XGrpKey[keyid - 1], keylen);
psetkeyparm->grpkey = 1;
break;
case _AES_:
- if (keyid < 1 || keyid > 2)
- return _FAIL;
+ if (keyid < 1 || keyid > 2) {
+ ret = _FAIL;
+ goto err_free_parm;
+ }
keylen = 16;
memcpy(psetkeyparm->key,
&psecuritypriv->XGrpKey[keyid - 1], keylen);
psetkeyparm->grpkey = 1;
break;
default:
- return _FAIL;
+ ret = _FAIL;
+ goto err_free_parm;
}
pcmd->cmdcode = _SetKey_CMD_;
pcmd->parmbuf = (u8 *)psetkeyparm;
@@ -1299,7 +1305,13 @@ sint r8712_set_key(struct _adapter *adapter,
pcmd->rspsz = 0;
_init_listhead(&pcmd->list);
r8712_enqueue_cmd(pcmdpriv, pcmd);
- return _SUCCESS;
+ return ret;
+
+err_free_parm:
+ kfree(psetkeyparm);
+err_free_cmd:
+ kfree(pcmd);
+ return ret;
}
/* adjust IEs for r8712_joinbss_cmd in WMM */
--
1.9.1
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: rtl8723au: fix potential leak in update_bcn_wps_ie()
Fix a potential leak in the error path of function update_bcn_wps_ie().
Move the affected input verification to the beginning of the function so
that it may return directly without leaking already allocated memory.
Detected by Coverity - CID 1077718.
Signed-off-by: Christian Engelmayer
---
v2: Added change suggested by Mateusz Guzik:
Move the check before allocating the memory instead of freeing the
resource afterwards in the error path.
Compile tested and applies against branch staging-next of tree
git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
---
drivers/staging/rtl8723au/core/rtw_ap.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8723au/core/rtw_ap.c
b/drivers/staging/rtl8723au/core/rtw_ap.c
index 9b31412..da028c535 100644
--- a/drivers/staging/rtl8723au/core/rtw_ap.c
+++ b/drivers/staging/rtl8723au/core/rtw_ap.c
@@ -1256,6 +1256,10 @@ static void update_bcn_wps_ie(struct rtw_adapter
*padapter)
DBG_8723A("%s\n", __func__);
+ pwps_ie_src = pmlmepriv->wps_beacon_ie;
+ if (pwps_ie_src == NULL)
+ return;
+
pwps_ie = rtw_get_wps_ie23a(ie+_FIXED_IE_LENGTH_,
ielen-_FIXED_IE_LENGTH_, NULL, &wps_ielen);
if (pwps_ie == NULL || wps_ielen == 0)
@@ -1274,10 +1278,6 @@ static void update_bcn_wps_ie(struct rtw_adapter
*padapter)
remainder_ielen);
}
- pwps_ie_src = pmlmepriv->wps_beacon_ie;
- if (pwps_ie_src == NULL)
- return;
-
wps_ielen = (uint)pwps_ie_src[1];/* to get ie data len */
if ((wps_offset+wps_ielen+2+remainder_ielen)<= MAX_IE_SZ)
{
--
1.9.1
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
