[U-Boot] [PATCH] dm: avoid dev->req_seq overflow
Since dev->req_seq value is initialized from "reg" property of fdt node, there is posibility, that address value contained in fdt is greater than INT_MAX, and then value in dev->req_seq is negative which led to probe() fail.
This patch fix this problem by ensuring that req_seq is positive, unless it's one of errno codes.
Signed-off-by: Robert Baldyga r.baldyga@samsung.com --- drivers/core/device.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/core/device.c b/drivers/core/device.c index 166b073..35ffce0 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -107,6 +107,8 @@ int device_bind(struct udevice *parent, struct driver *drv, const char *name, * when the device is probed. */ dev->req_seq = fdtdec_get_int(gd->fdt_blob, of_offset, "reg", -1); + if (!IS_ERR_VALUE(dev->req_seq)) + dev->req_seq &= INT_MAX; dev->seq = -1; if (uc->uc_drv->name && of_offset != -1) { fdtdec_get_alias_seq(gd->fdt_blob, uc->uc_drv->name, of_offset,
Hi Robert,
On 18 September 2014 09:13, Robert Baldyga r.baldyga@samsung.com wrote:
Since dev->req_seq value is initialized from "reg" property of fdt node, there is posibility, that address value contained in fdt is greater than INT_MAX, and then value in dev->req_seq is negative which led to probe() fail.
This patch fix this problem by ensuring that req_seq is positive, unless it's one of errno codes.
Wouldn't this be a bug in the device tree file? What does it mean to have a -ve value?
Signed-off-by: Robert Baldyga r.baldyga@samsung.com
drivers/core/device.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/core/device.c b/drivers/core/device.c index 166b073..35ffce0 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -107,6 +107,8 @@ int device_bind(struct udevice *parent, struct driver *drv, const char *name, * when the device is probed. */ dev->req_seq = fdtdec_get_int(gd->fdt_blob, of_offset, "reg", -1);
if (!IS_ERR_VALUE(dev->req_seq))dev->req_seq &= INT_MAX; dev->seq = -1; if (uc->uc_drv->name && of_offset != -1) { fdtdec_get_alias_seq(gd->fdt_blob, uc->uc_drv->name,of_offset,
1.9.1
Regards, Simon
On 09/18/2014 08:00 PM, Simon Glass wrote:
Hi Robert,
On 18 September 2014 09:13, Robert Baldyga <r.baldyga@samsung.com mailto:r.baldyga@samsung.com> wrote:
Since dev->req_seq value is initialized from "reg" property of fdt node, there is posibility, that address value contained in fdt is greater than INT_MAX, and then value in dev->req_seq is negative which led to probe() fail. This patch fix this problem by ensuring that req_seq is positive, unless it's one of errno codes.Wouldn't this be a bug in the device tree file? What does it mean to have a -ve value?
Device tree seems to be ok. We have:
pinctrl0: pinctrl@e0200000 { compatible = "samsung,s5pc110-pinctrl"; reg = <0xe0200000 0x1000>; };
So when we take address from "reg" as dev->req_seq, then value 0xe0200000 after casting to int gives -534773760. Function uclass_resolve_seq() returns it as proper seq number, because it's unique. But then in file drivers/core/device.c, in function device_probe() we have:
seq = uclass_resolve_seq(dev); if (seq < 0) { ret = seq; goto fail; }
And it will obviously fail.
Using "reg" value as req_seq doesn't work when this value is greater than INT_MAX.
Signed-off-by: Robert Baldyga <r.baldyga@samsung.com <mailto:r.baldyga@samsung.com>> --- drivers/core/device.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/core/device.c b/drivers/core/device.c index 166b073..35ffce0 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -107,6 +107,8 @@ int device_bind(struct udevice *parent, struct driver *drv, const char *name, * when the device is probed. */ dev->req_seq = fdtdec_get_int(gd->fdt_blob, of_offset, "reg", -1); + if (!IS_ERR_VALUE(dev->req_seq)) + dev->req_seq &= INT_MAX; dev->seq = -1; if (uc->uc_drv->name && of_offset != -1) { fdtdec_get_alias_seq(gd->fdt_blob, uc->uc_drv->name, of_offset, -- 1.9.1
Thanks, Robert Baldyga
Hi Robert,
On 18 September 2014 23:25, Robert Baldyga r.baldyga@samsung.com wrote:
On 09/18/2014 08:00 PM, Simon Glass wrote:
Hi Robert,
On 18 September 2014 09:13, Robert Baldyga <r.baldyga@samsung.com mailto:r.baldyga@samsung.com> wrote:
Since dev->req_seq value is initialized from "reg" property of fdt node, there is posibility, that address value contained in fdt is greater than INT_MAX, and then value in dev->req_seq is negative which led to probe() fail. This patch fix this problem by ensuring that req_seq is positive, unless it's one of errno codes.Wouldn't this be a bug in the device tree file? What does it mean to have a -ve value?
Device tree seems to be ok. We have:
pinctrl0: pinctrl@e0200000 { compatible = "samsung,s5pc110-pinctrl"; reg = <0xe0200000 0x1000>; };So when we take address from "reg" as dev->req_seq, then value 0xe0200000 after casting to int gives -534773760. Function uclass_resolve_seq() returns it as proper seq number, because it's unique. But then in file drivers/core/device.c, in function device_probe() we have:
seq = uclass_resolve_seq(dev); if (seq < 0) { ret = seq; goto fail; }And it will obviously fail.
Using "reg" value as req_seq doesn't work when this value is greater than INT_MAX.
OK I see. Thanks for the clear explanation. We don't really want req_seq for things that are not buses, but there is no real concept of that in DM at this stage. Let's see how things pan out. For now, this patch is a good solution.
Acked-by: Simon Glass sjg@chromium.org
Regards, Simon
participants (2)
-
Robert Baldyga -
Simon Glass