| Message ID | 1470625113-29275-2-git-send-email-oohall@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Aug 8, 2016 at 12:28 PM, Oliver O'Halloran <oohall@gmail.com> wrote: > NVRAM configuration strings are required to be NUL terminated and unused > data bytes in the partition should be set to NUL. Badly behaved system > software may not do this so same sanity checking is required. Ensuring > that the final data byte in a partition is a NUL should be sufficent. Spelling (sufficient). > > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> > --- > core/nvram-format.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/core/nvram-format.c b/core/nvram-format.c > index a81663ceb35f..be8d77f8394b 100644 > --- a/core/nvram-format.c > +++ b/core/nvram-format.c > @@ -155,6 +155,20 @@ int nvram_check(void *nvram_image, const uint32_t nvram_size) > prerror("NVRAM: Skiboot private partition " > "not found !\n"); > goto failed; > + } else { > + /* > + * The OF NVRAM format requires config strings to be NUL > + * terminated and unused memory to be set ot zero. Well behaved Typo > + * software should ensure this is done for us, but we should > + * always check. > + */ > + const char *c = (const char *) skiboot_part_hdr + > + skiboot_part_hdr->len * 16 - 1; c is going to point to the last byte of the partition? We could call it 'last_byte'. Is ->len user controlled? Do we need to make sure it's not pointing past the end of something? > + > + if (*c != 0) { > + prerror("NVRAM: Skiboot private partition is not NUL terminated"); > + goto failed; > + } > } > > prlog(PR_INFO, "NVRAM: Layout appears sane\n"); > -- > 2.5.5 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
On Mon, Aug 8, 2016 at 6:00 PM, Joel Stanley <joel@jms.id.au> wrote: > On Mon, Aug 8, 2016 at 12:28 PM, Oliver O'Halloran <oohall@gmail.com> wrote: >> NVRAM configuration strings are required to be NUL terminated and unused >> data bytes in the partition should be set to NUL. Badly behaved system >> software may not do this so same sanity checking is required. Ensuring >> that the final data byte in a partition is a NUL should be sufficent. > > Spelling (sufficient). fixed > > >> >> Signed-off-by: Oliver O'Halloran <oohall@gmail.com> >> --- >> core/nvram-format.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/core/nvram-format.c b/core/nvram-format.c >> index a81663ceb35f..be8d77f8394b 100644 >> --- a/core/nvram-format.c >> +++ b/core/nvram-format.c >> @@ -155,6 +155,20 @@ int nvram_check(void *nvram_image, const uint32_t nvram_size) >> prerror("NVRAM: Skiboot private partition " >> "not found !\n"); >> goto failed; >> + } else { >> + /* >> + * The OF NVRAM format requires config strings to be NUL >> + * terminated and unused memory to be set ot zero. Well behaved > > Typo fixed > >> + * software should ensure this is done for us, but we should >> + * always check. >> + */ >> + const char *c = (const char *) skiboot_part_hdr + >> + skiboot_part_hdr->len * 16 - 1; > > c is going to point to the last byte of the partition? We could call > it 'last_byte'. That's a better name. > Is ->len user controlled? Do we need to make sure it's not pointing > past the end of something? It is user controlled, but it's validated earlier in that function while iterating through the nvram partitions. > >> + >> + if (*c != 0) { >> + prerror("NVRAM: Skiboot private partition is not NUL terminated"); >> + goto failed; >> + } >> } >> >> prlog(PR_INFO, "NVRAM: Layout appears sane\n"); >> -- >> 2.5.5 >> >> _______________________________________________ >> Skiboot mailing list >> Skiboot@lists.ozlabs.org >> https://lists.ozlabs.org/listinfo/skiboot
diff --git a/core/nvram-format.c b/core/nvram-format.c index a81663ceb35f..be8d77f8394b 100644 --- a/core/nvram-format.c +++ b/core/nvram-format.c @@ -155,6 +155,20 @@ int nvram_check(void *nvram_image, const uint32_t nvram_size) prerror("NVRAM: Skiboot private partition " "not found !\n"); goto failed; + } else { + /* + * The OF NVRAM format requires config strings to be NUL + * terminated and unused memory to be set ot zero. Well behaved + * software should ensure this is done for us, but we should + * always check. + */ + const char *c = (const char *) skiboot_part_hdr + + skiboot_part_hdr->len * 16 - 1; + + if (*c != 0) { + prerror("NVRAM: Skiboot private partition is not NUL terminated"); + goto failed; + } } prlog(PR_INFO, "NVRAM: Layout appears sane\n");
NVRAM configuration strings are required to be NUL terminated and unused data bytes in the partition should be set to NUL. Badly behaved system software may not do this so same sanity checking is required. Ensuring that the final data byte in a partition is a NUL should be sufficent. Signed-off-by: Oliver O'Halloran <oohall@gmail.com> --- core/nvram-format.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)