Hi--
Here are a few corrections for the source code.
On 9/4/20 12:40 PM, Michael Witten wrote:
The following patch improves the quality and correctness of the openprom code.
I have neither a machine to test the result nor a toolchain to compile it, and
that is why it is listed currently as an "RFC". Nonetheless, I believe those
what is your host build system?
https://mirrors.edge.kernel.org/pub/tools/crosstool/
who do have these tools will find the proposed changes useful; I hope you will
help me iterate this patch into something worthy of merging (or use it as the
basis for your own changes).
Sincerely,
Michael Witten
--8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<--
...
Signed-off-by: Michael Witten <mfwitten@xxxxxxxxx>
---
arch/sparc/include/asm/prom.h | 2 +-
arch/sparc/kernel/prom_common.c | 14 +--
drivers/sbus/char/openprom.c | 263 ++++++++++++++++++++++++++++------------
3 files changed, 194 insertions(+), 85 deletions(-)
diff --git a/drivers/sbus/char/openprom.c b/drivers/sbus/char/openprom.c
index 30b9751aad30..9bc2877aa09a 100644
--- a/drivers/sbus/char/openprom.c
+++ b/drivers/sbus/char/openprom.c
@@ -120,69 +154,109 @@ static int getstrings(struct openpromio __user *info, struct openpromio **opp_p)
-static int opromgetprop(void __user *argp, struct device_node *dp, struct openpromio *op, int bufsize)
+static int opromgetprop(void __user *argp, struct device_node *dp, struct openpromio *op, const size_t bufsize)
{
const void *pval;
- int len;
+ int pval_size;
if (!dp ||
- !(pval = of_get_property(dp, op->oprom_array, &len)) ||
- len <= 0 || len > bufsize)
+ !(pval = of_get_property(dp, op->oprom_array, &pval_size)) ||
+ pval_size <= 0 || pval_size > bufsize) {
+ #ifdef CONFIG_DEBUG_KERNEL
+ if (WARN_ON(op->oprom_size))
+ op->oprom_size = 0;
+ #endif
return copyout(argp, op, sizeof(int));
+ }
+
+ op->oprom_size = pval_size;
+ memcpy(op->oprom_array, pval, pval_size);
- memcpy(op->oprom_array, pval, len);
- op->oprom_array[len] = '\0';
- op->oprom_size = len;
+ #ifdef CONFIG_DEBUG_KERNEL
+ // If the buffer is larger than needed, then
+ // the contents should be nul-terminated in
+ // case the PROM erroneously produces a string
+ // that is not nul-terminated.
+ if (pval_size < bufsize)
+ char *const end = op->oprom_array + pval_size;
+ if (WARN_ON(*end))
+ *end = 0;
Missing braces above?
+ #endif
return copyout(argp, op, sizeof(int) + bufsize);
}
-static int opromnxtprop(void __user *argp, struct device_node *dp, struct openpromio *op, int bufsize)
+static int opromnxtprop(void __user *argp, struct device_node *dp, struct openpromio *op, const size_t bufsize)
{
struct property *prop;
- int len;
+ size_t name_size;
+ unsigned int uint_name_size;
if (!dp)
- return copyout(argp, op, sizeof(int));
+ goto nothing;
+
if (op->oprom_array[0] == '\0') {
prop = dp->properties;
- if (!prop)
- return copyout(argp, op, sizeof(int));
- len = strlen(prop->name);
} else {
prop = of_find_property(dp, op->oprom_array, NULL);
+ if (prop)
+ prop = prop->next;
+ }
- if (!prop ||
- !prop->next ||
- (len = strlen(prop->next->name)) + 1 > bufsize)
- return copyout(argp, op, sizeof(int));
+ if (!prop)
+ goto nothing;
- prop = prop->next;
- }
+ name_size = 1 + strlen(prop->name);
+
+ uint_name_size = name_size;
+ if (unlikely(uint_name_size != name_size))
+ goto nothing; // overflow
+
+ if (name_size > bufsize)
+ goto nothing;
- memcpy(op->oprom_array, prop->name, len);
- op->oprom_array[len] = '\0';
- op->oprom_size = ++len;
+ memcpy(op->oprom_array, prop->name, name_size);
+ op->oprom_size = uint_name_size;
return copyout(argp, op, sizeof(int) + bufsize);
+
+nothing:
+ #ifdef CONFIG_DEBUG_KERNEL
+ if (WARN_ON(op->oprom_size))
+ op-oprom_size = 0;
op->oprom_size = 0;
+ #endif
+ return copyout(argp, op, sizeof(int));
}
@@ -301,6 +384,12 @@ static long openprom_sunos_ioctl(struct file * file,
else
bufsize = copyin(argp, &opp);
+ #ifdef CONFIG_DEBUG_KERNEL
+ if (WARN_ON(bufsize == 0))
+ bufsize = -EFAULT;
+ #enif
#endif
+
+ static_assert(LONG_MIN <= -(SIZE_MAX/2)-1);
if (bufsize < 0)
return bufsize;
HTH.
--
~Randy
