Re: [PATCH V2] pinctrl: mvebu: prevent walking off the end of group array

[David Woodhouse]

On Wed, 2013-03-13 at 17:48 +, Jason Cooper wrote:
> From: David Woodhouse 
> 
> While investigating (ab)use of krealloc, David found this bug.  It's
> unlikely to occur, but now we detect the condition and error out
> appropriately.
> 
> Signed-off-by: David Woodhouse 
> Signed-off-by: Jason Cooper 
> ---
> Changes from v1:
>  - correct typo (s/ nt / int /) I should've caught before sending.
> 
> David, please double check that this is as you intended.  I had to hand-jam it
> in due to some peculiarities on my side.

Looks fine; thanks.

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation


smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH V2] pinctrl: mvebu: prevent walking off the end of group array

[David Woodhouse]

On Wed, 2013-03-13 at 17:48 +, Jason Cooper wrote:
 From: David Woodhouse dw...@infradead.org
 
 While investigating (ab)use of krealloc, David found this bug.  It's
 unlikely to occur, but now we detect the condition and error out
 appropriately.
 
 Signed-off-by: David Woodhouse david.woodho...@intel.com
 Signed-off-by: Jason Cooper ja...@lakedaemon.net
 ---
 Changes from v1:
  - correct typo (s/ nt / int /) I should've caught before sending.
 
 David, please double check that this is as you intended.  I had to hand-jam it
 in due to some peculiarities on my side.

Looks fine; thanks.

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation


smime.p7s
Description: S/MIME cryptographic signature

[PATCH V2] pinctrl: mvebu: prevent walking off the end of group array

[Jason Cooper]

From: David Woodhouse 

While investigating (ab)use of krealloc, David found this bug.  It's
unlikely to occur, but now we detect the condition and error out
appropriately.

Signed-off-by: David Woodhouse 
Signed-off-by: Jason Cooper 
---
Changes from v1:
 - correct typo (s/ nt / int /) I should've caught before sending.

David, please double check that this is as you intended.  I had to hand-jam it
in due to some peculiarities on my side.

 drivers/pinctrl/mvebu/pinctrl-mvebu.c | 27 ---
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.c 
b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
index c689c04..5db5fad 100644
--- a/drivers/pinctrl/mvebu/pinctrl-mvebu.c
+++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
@@ -478,16 +478,21 @@ static struct pinctrl_ops mvebu_pinctrl_ops = {
.dt_free_map = mvebu_pinctrl_dt_free_map,
 };
 
-static int _add_function(struct mvebu_pinctrl_function *funcs, const char 
*name)
+static int _add_function(struct mvebu_pinctrl_function *funcs, int nr_funcs,
+   const char *name)
 {
-   while (funcs->num_groups) {
+   while (nr_funcs && funcs->num_groups) {
/* function already there */
if (strcmp(funcs->name, name) == 0) {
funcs->num_groups++;
return -EEXIST;
}
funcs++;
+   nr_funcs--;
}
+   if (!nr_funcs)
+   return -EOVERFLOW;
+
funcs->name = name;
funcs->num_groups = 1;
return 0;
@@ -501,7 +506,7 @@ static int mvebu_pinctrl_build_functions(struct 
platform_device *pdev,
int n, s;
 
/* we allocate functions for number of pins and hope
-* there are less unique functions than pins available */
+* there are fewer unique functions than pins available */
funcs = devm_kzalloc(>dev, pctl->desc.npins *
 sizeof(struct mvebu_pinctrl_function), GFP_KERNEL);
if (!funcs)
@@ -510,26 +515,26 @@ static int mvebu_pinctrl_build_functions(struct 
platform_device *pdev,
for (n = 0; n < pctl->num_groups; n++) {
struct mvebu_pinctrl_group *grp = >groups[n];
for (s = 0; s < grp->num_settings; s++) {
+   int ret;
+
/* skip unsupported settings on this variant */
if (pctl->variant &&
!(pctl->variant & grp->settings[s].variant))
continue;
 
/* check for unique functions and count groups */
-   if (_add_function(funcs, grp->settings[s].name))
+   ret = _add_function(funcs, pctl->desc.npins,
+   grp->settings[s].name);
+   if (ret == -EOVERFLOW)
+   dev_err(>dev,
+   "More functions than pins(%d)\n",
+   pctl->desc.npins);
continue;
 
num++;
}
}
 
-   /* with the number of unique functions and it's groups known,
-  reallocate functions and assign group names */
-   funcs = krealloc(funcs, num * sizeof(struct mvebu_pinctrl_function),
-GFP_KERNEL);
-   if (!funcs)
-   return -ENOMEM;
-
pctl->num_functions = num;
pctl->functions = funcs;
 
-- 
1.8.1.5

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

[PATCH V2] pinctrl: mvebu: prevent walking off the end of group array

[Jason Cooper]

From: David Woodhouse dw...@infradead.org

While investigating (ab)use of krealloc, David found this bug.  It's
unlikely to occur, but now we detect the condition and error out
appropriately.

Signed-off-by: David Woodhouse david.woodho...@intel.com
Signed-off-by: Jason Cooper ja...@lakedaemon.net
---
Changes from v1:
 - correct typo (s/ nt / int /) I should've caught before sending.

David, please double check that this is as you intended.  I had to hand-jam it
in due to some peculiarities on my side.

 drivers/pinctrl/mvebu/pinctrl-mvebu.c | 27 ---
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.c 
b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
index c689c04..5db5fad 100644
--- a/drivers/pinctrl/mvebu/pinctrl-mvebu.c
+++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
@@ -478,16 +478,21 @@ static struct pinctrl_ops mvebu_pinctrl_ops = {
.dt_free_map = mvebu_pinctrl_dt_free_map,
 };
 
-static int _add_function(struct mvebu_pinctrl_function *funcs, const char 
*name)
+static int _add_function(struct mvebu_pinctrl_function *funcs, int nr_funcs,
+   const char *name)
 {
-   while (funcs-num_groups) {
+   while (nr_funcs  funcs-num_groups) {
/* function already there */
if (strcmp(funcs-name, name) == 0) {
funcs-num_groups++;
return -EEXIST;
}
funcs++;
+   nr_funcs--;
}
+   if (!nr_funcs)
+   return -EOVERFLOW;
+
funcs-name = name;
funcs-num_groups = 1;
return 0;
@@ -501,7 +506,7 @@ static int mvebu_pinctrl_build_functions(struct 
platform_device *pdev,
int n, s;
 
/* we allocate functions for number of pins and hope
-* there are less unique functions than pins available */
+* there are fewer unique functions than pins available */
funcs = devm_kzalloc(pdev-dev, pctl-desc.npins *
 sizeof(struct mvebu_pinctrl_function), GFP_KERNEL);
if (!funcs)
@@ -510,26 +515,26 @@ static int mvebu_pinctrl_build_functions(struct 
platform_device *pdev,
for (n = 0; n  pctl-num_groups; n++) {
struct mvebu_pinctrl_group *grp = pctl-groups[n];
for (s = 0; s  grp-num_settings; s++) {
+   int ret;
+
/* skip unsupported settings on this variant */
if (pctl-variant 
!(pctl-variant  grp-settings[s].variant))
continue;
 
/* check for unique functions and count groups */
-   if (_add_function(funcs, grp-settings[s].name))
+   ret = _add_function(funcs, pctl-desc.npins,
+   grp-settings[s].name);
+   if (ret == -EOVERFLOW)
+   dev_err(pdev-dev,
+   More functions than pins(%d)\n,
+   pctl-desc.npins);
continue;
 
num++;
}
}
 
-   /* with the number of unique functions and it's groups known,
-  reallocate functions and assign group names */
-   funcs = krealloc(funcs, num * sizeof(struct mvebu_pinctrl_function),
-GFP_KERNEL);
-   if (!funcs)
-   return -ENOMEM;
-
pctl-num_functions = num;
pctl-functions = funcs;
 
-- 
1.8.1.5

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