* [PATCH] loads: Block writes into LMB reserved areas of U-Boot
@ 2021-10-10 21:52 marek.vasut
2021-10-14 15:10 ` Simon Glass
2021-10-26 13:34 ` Tom Rini
0 siblings, 2 replies; 6+ messages in thread
From: marek.vasut @ 2021-10-10 21:52 UTC (permalink / raw)
To: u-boot; +Cc: Marek Vasut, Simon Glass, Tom Rini
From: Marek Vasut <marek.vasut+renesas@gmail.com>
The loads srec loading may overwrite piece of U-Boot accidentally.
Prevent that by using LMB to detect whether upcoming write would
overwrite piece of reserved U-Boot code, and if that is the case,
abort the srec loading.
Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@konsulko.com>
---
cmd/load.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/cmd/load.c b/cmd/load.c
index 249ebd4ae0..7e4a552d90 100644
--- a/cmd/load.c
+++ b/cmd/load.c
@@ -16,6 +16,7 @@
#include <exports.h>
#include <flash.h>
#include <image.h>
+#include <lmb.h>
#include <mapmem.h>
#include <net.h>
#include <s_record.h>
@@ -137,6 +138,7 @@ static int do_load_serial(struct cmd_tbl *cmdtp, int flag, int argc,
static ulong load_serial(long offset)
{
+ struct lmb lmb;
char record[SREC_MAXRECLEN + 1]; /* buffer for one S-Record */
char binbuf[SREC_MAXBINLEN]; /* buffer for binary data */
int binlen; /* no. of data bytes in S-Rec. */
@@ -147,6 +149,9 @@ static ulong load_serial(long offset)
ulong start_addr = ~0;
ulong end_addr = 0;
int line_count = 0;
+ long ret;
+
+ lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
while (read_record(record, SREC_MAXRECLEN + 1) >= 0) {
type = srec_decode(record, &binlen, &addr, binbuf);
@@ -172,7 +177,14 @@ static ulong load_serial(long offset)
} else
#endif
{
+ ret = lmb_reserve(&lmb, store_addr, binlen);
+ if (ret) {
+ printf("\nCannot overwrite reserved area (%08lx..%08lx)\n",
+ store_addr, store_addr + binlen);
+ return ret;
+ }
memcpy((char *)(store_addr), binbuf, binlen);
+ lmb_free(&lmb, store_addr, binlen);
}
if ((store_addr) < start_addr)
start_addr = store_addr;
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] loads: Block writes into LMB reserved areas of U-Boot
2021-10-10 21:52 [PATCH] loads: Block writes into LMB reserved areas of U-Boot marek.vasut
@ 2021-10-14 15:10 ` Simon Glass
2021-10-15 14:23 ` Marek Vasut
2021-10-26 13:34 ` Tom Rini
1 sibling, 1 reply; 6+ messages in thread
From: Simon Glass @ 2021-10-14 15:10 UTC (permalink / raw)
To: Marek Vasut; +Cc: U-Boot Mailing List, Marek Vasut, Tom Rini
Hi Marek,
On Sun, 10 Oct 2021 at 15:52, <marek.vasut@gmail.com> wrote:
>
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>
> The loads srec loading may overwrite piece of U-Boot accidentally.
> Prevent that by using LMB to detect whether upcoming write would
> overwrite piece of reserved U-Boot code, and if that is the case,
> abort the srec loading.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> ---
> cmd/load.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/cmd/load.c b/cmd/load.c
> index 249ebd4ae0..7e4a552d90 100644
> --- a/cmd/load.c
> +++ b/cmd/load.c
> @@ -16,6 +16,7 @@
> #include <exports.h>
> #include <flash.h>
> #include <image.h>
> +#include <lmb.h>
> #include <mapmem.h>
> #include <net.h>
> #include <s_record.h>
> @@ -137,6 +138,7 @@ static int do_load_serial(struct cmd_tbl *cmdtp, int flag, int argc,
>
> static ulong load_serial(long offset)
> {
> + struct lmb lmb;
> char record[SREC_MAXRECLEN + 1]; /* buffer for one S-Record */
> char binbuf[SREC_MAXBINLEN]; /* buffer for binary data */
> int binlen; /* no. of data bytes in S-Rec. */
> @@ -147,6 +149,9 @@ static ulong load_serial(long offset)
> ulong start_addr = ~0;
> ulong end_addr = 0;
> int line_count = 0;
> + long ret;
> +
> + lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
>
> while (read_record(record, SREC_MAXRECLEN + 1) >= 0) {
> type = srec_decode(record, &binlen, &addr, binbuf);
> @@ -172,7 +177,14 @@ static ulong load_serial(long offset)
> } else
> #endif
> {
> + ret = lmb_reserve(&lmb, store_addr, binlen);
> + if (ret) {
> + printf("\nCannot overwrite reserved area (%08lx..%08lx)\n",
> + store_addr, store_addr + binlen);
> + return ret;
> + }
> memcpy((char *)(store_addr), binbuf, binlen);
> + lmb_free(&lmb, store_addr, binlen);
> }
> if ((store_addr) < start_addr)
> start_addr = store_addr;
> --
> 2.33.0
>
Reviewed-by: Simon Glass <sjg@chromium.org>
This code looks OK but I don't know what lmb_reserve() and lmb_free()
do. Can you add comments to the header file?
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] loads: Block writes into LMB reserved areas of U-Boot
2021-10-14 15:10 ` Simon Glass
@ 2021-10-15 14:23 ` Marek Vasut
2021-10-15 16:09 ` Tom Rini
0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2021-10-15 14:23 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List, Marek Vasut, Tom Rini
On 10/14/21 5:10 PM, Simon Glass wrote:
[...]
>> @@ -137,6 +138,7 @@ static int do_load_serial(struct cmd_tbl *cmdtp, int flag, int argc,
>>
>> static ulong load_serial(long offset)
>> {
>> + struct lmb lmb;
>> char record[SREC_MAXRECLEN + 1]; /* buffer for one S-Record */
>> char binbuf[SREC_MAXBINLEN]; /* buffer for binary data */
>> int binlen; /* no. of data bytes in S-Rec. */
>> @@ -147,6 +149,9 @@ static ulong load_serial(long offset)
>> ulong start_addr = ~0;
>> ulong end_addr = 0;
>> int line_count = 0;
>> + long ret;
>> +
>> + lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
>>
>> while (read_record(record, SREC_MAXRECLEN + 1) >= 0) {
>> type = srec_decode(record, &binlen, &addr, binbuf);
>> @@ -172,7 +177,14 @@ static ulong load_serial(long offset)
>> } else
>> #endif
>> {
>> + ret = lmb_reserve(&lmb, store_addr, binlen);
>> + if (ret) {
>> + printf("\nCannot overwrite reserved area (%08lx..%08lx)\n",
>> + store_addr, store_addr + binlen);
>> + return ret;
>> + }
>> memcpy((char *)(store_addr), binbuf, binlen);
>> + lmb_free(&lmb, store_addr, binlen);
>> }
>> if ((store_addr) < start_addr)
>> start_addr = store_addr;
>> --
>> 2.33.0
>>
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> This code looks OK but I don't know what lmb_reserve() and lmb_free()
> do. Can you add comments to the header file?
Not here, the entire LMB stuff needs (better) documentation, that's
where (all) such clarification should go.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] loads: Block writes into LMB reserved areas of U-Boot
2021-10-15 14:23 ` Marek Vasut
@ 2021-10-15 16:09 ` Tom Rini
2021-10-15 16:30 ` Marek Vasut
0 siblings, 1 reply; 6+ messages in thread
From: Tom Rini @ 2021-10-15 16:09 UTC (permalink / raw)
To: Marek Vasut; +Cc: Simon Glass, U-Boot Mailing List, Marek Vasut
[-- Attachment #1: Type: text/plain, Size: 2306 bytes --]
On Fri, Oct 15, 2021 at 04:23:27PM +0200, Marek Vasut wrote:
> On 10/14/21 5:10 PM, Simon Glass wrote:
> [...]
> > > @@ -137,6 +138,7 @@ static int do_load_serial(struct cmd_tbl *cmdtp, int flag, int argc,
> > >
> > > static ulong load_serial(long offset)
> > > {
> > > + struct lmb lmb;
> > > char record[SREC_MAXRECLEN + 1]; /* buffer for one S-Record */
> > > char binbuf[SREC_MAXBINLEN]; /* buffer for binary data */
> > > int binlen; /* no. of data bytes in S-Rec. */
> > > @@ -147,6 +149,9 @@ static ulong load_serial(long offset)
> > > ulong start_addr = ~0;
> > > ulong end_addr = 0;
> > > int line_count = 0;
> > > + long ret;
> > > +
> > > + lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
> > >
> > > while (read_record(record, SREC_MAXRECLEN + 1) >= 0) {
> > > type = srec_decode(record, &binlen, &addr, binbuf);
> > > @@ -172,7 +177,14 @@ static ulong load_serial(long offset)
> > > } else
> > > #endif
> > > {
> > > + ret = lmb_reserve(&lmb, store_addr, binlen);
> > > + if (ret) {
> > > + printf("\nCannot overwrite reserved area (%08lx..%08lx)\n",
> > > + store_addr, store_addr + binlen);
> > > + return ret;
> > > + }
> > > memcpy((char *)(store_addr), binbuf, binlen);
> > > + lmb_free(&lmb, store_addr, binlen);
> > > }
> > > if ((store_addr) < start_addr)
> > > start_addr = store_addr;
> > > --
> > > 2.33.0
> > >
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > This code looks OK but I don't know what lmb_reserve() and lmb_free()
> > do. Can you add comments to the header file?
>
> Not here, the entire LMB stuff needs (better) documentation, that's where
> (all) such clarification should go.
Is that you saying you'll do such a follow-up patch, given you've
touched this code the most of late?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] loads: Block writes into LMB reserved areas of U-Boot
2021-10-15 16:09 ` Tom Rini
@ 2021-10-15 16:30 ` Marek Vasut
0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2021-10-15 16:30 UTC (permalink / raw)
To: Tom Rini; +Cc: Simon Glass, U-Boot Mailing List
On 10/15/21 6:09 PM, Tom Rini wrote:
[...]
>>> This code looks OK but I don't know what lmb_reserve() and lmb_free()
>>> do. Can you add comments to the header file?
>>
>> Not here, the entire LMB stuff needs (better) documentation, that's where
>> (all) such clarification should go.
>
> Is that you saying you'll do such a follow-up patch, given you've
> touched this code the most of late?
Maybe, I won't promise you anything except I add it to my roll of todo
paper.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] loads: Block writes into LMB reserved areas of U-Boot
2021-10-10 21:52 [PATCH] loads: Block writes into LMB reserved areas of U-Boot marek.vasut
2021-10-14 15:10 ` Simon Glass
@ 2021-10-26 13:34 ` Tom Rini
1 sibling, 0 replies; 6+ messages in thread
From: Tom Rini @ 2021-10-26 13:34 UTC (permalink / raw)
To: marek.vasut; +Cc: u-boot, Marek Vasut, Simon Glass
[-- Attachment #1: Type: text/plain, Size: 602 bytes --]
On Sun, Oct 10, 2021 at 11:52:41PM +0200, marek.vasut@gmail.com wrote:
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>
> The loads srec loading may overwrite piece of U-Boot accidentally.
> Prevent that by using LMB to detect whether upcoming write would
> overwrite piece of reserved U-Boot code, and if that is the case,
> abort the srec loading.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
Applied to u-boot/master, thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-26 13:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-10 21:52 [PATCH] loads: Block writes into LMB reserved areas of U-Boot marek.vasut
2021-10-14 15:10 ` Simon Glass
2021-10-15 14:23 ` Marek Vasut
2021-10-15 16:09 ` Tom Rini
2021-10-15 16:30 ` Marek Vasut
2021-10-26 13:34 ` Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).