[U-Boot] [PATCH 1/3] ubifs: BUG realpath string must be ended with NULL
If the memory used to copy the link_make is "dirty" the string wont be ended with NULL, throwing out multiple memory bugs.
Signed-off-by: Ricardo Ribalda Delgado ricardo.ribalda@uam.es --- fs/ubifs/ubifs.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 32f9ff8..427d84a 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -641,6 +641,7 @@ int ubifs_load(char *filename, u32 addr, u32 size) ui = ubifs_inode(inode); if (((inode->i_mode & S_IFMT) == S_IFLNK) && ui->data_len) { memcpy(link_name, ui->data, ui->data_len); + link_name[ui->data_len] = '\0'; printf("%s is linked to %s!\n", filename, link_name); ubifs_iput(inode);
Separate gunzip in
gunzip: Find the end of the header and call zunzip. zunzip: Inflate gunzip block without header.
UBI fs blocks can be compresed in lzo, zlib or no-compression. The current implementation of u-boot supported all the compressions but there was a bug in the implementation of the zlib blocks.
UBIFS's Zlib blocks do not have header but they were compressed using gunzip, a function used to decompress gunzip files/sectors with a header.
This patch adds a new function zunzip that uncompress a zlib block with no header.
Signed-off-by: Ricardo Ribalda Delgado ricardo.ribalda@uam.es --- v2: Better doc
lib_generic/gunzip.c | 27 ++++++++++++++++++++------- 1 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/lib_generic/gunzip.c b/lib_generic/gunzip.c index 01a4031..d59a448 100644 --- a/lib_generic/gunzip.c +++ b/lib_generic/gunzip.c @@ -39,6 +39,8 @@ int gunzip(void *, int, unsigned char *, unsigned long *); void *zalloc(void *, unsigned, unsigned); void zfree(void *, void *, unsigned); +int zunzip(void *dst, int dstlen, unsigned char *src, unsigned long *lenp, + int stoponerr, int offset);
void *zalloc(void *x, unsigned items, unsigned size) { @@ -59,8 +61,7 @@ void zfree(void *x, void *addr, unsigned nb)
int gunzip(void *dst, int dstlen, unsigned char *src, unsigned long *lenp) { - z_stream s; - int r, i, flags; + int i, flags;
/* skip header */ i = 10; @@ -84,6 +85,18 @@ int gunzip(void *dst, int dstlen, unsigned char *src, unsigned long *lenp) return (-1); }
+ return zunzip(dst, dstlen, src, lenp, 1, i); +} + +/* + * Uncompress blocks compressed with zlib without headers + */ +int zunzip(void *dst, int dstlen, unsigned char *src, unsigned long *lenp, + int stoponerr, int offset) +{ + z_stream s; + int r; + s.zalloc = zalloc; s.zfree = zfree; #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) @@ -95,14 +108,14 @@ int gunzip(void *dst, int dstlen, unsigned char *src, unsigned long *lenp) r = inflateInit2(&s, -MAX_WBITS); if (r != Z_OK) { printf ("Error: inflateInit2() returned %d\n", r); - return (-1); + return -1; } - s.next_in = src + i; - s.avail_in = *lenp - i; + s.next_in = src + offset; + s.avail_in = *lenp - offset; s.next_out = dst; s.avail_out = dstlen; r = inflate(&s, Z_FINISH); - if (r != Z_STREAM_END) { + if ((r != Z_STREAM_END) && (stoponerr==1)) { printf ("Error: inflate() returned %d\n", r); inflateEnd(&s); return (-1); @@ -110,5 +123,5 @@ int gunzip(void *dst, int dstlen, unsigned char *src, unsigned long *lenp) *lenp = s.next_out - (unsigned char *) dst; inflateEnd(&s);
- return (0); + return 0; }
Blocks compressed with zlib dont have the full gzip header.
Without this patch, block compressed with zlib cannot be readed!
Signed-off-by: Ricardo Ribalda Delgado ricardo.ribalda@uam.es --- v3: rename patch 2-> patch 3 v2: remove unused parts.. fs/ubifs/ubifs.c | 7 +++++-- fs/ubifs/ubifs.h | 2 -- 2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 427d84a..739fb04 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -24,6 +24,7 @@ */
#include "ubifs.h" +#include <u-boot/zlib.h>
#if !defined(CONFIG_SYS_64BIT_VSPRINTF) #warning Please define CONFIG_SYS_64BIT_VSPRINTF for correct output! @@ -33,15 +34,17 @@ DECLARE_GLOBAL_DATA_PTR;
/* compress.c */
+int zunzip(void *dst, int dstlen, unsigned char *src, unsigned long *lenp, + int stoponerr, int offset); /* - * We need a wrapper for gunzip() because the parameters are + * We need a wrapper for zunzip() because the parameters are * incompatible with the lzo decompressor. */ static int gzip_decompress(const unsigned char *in, size_t in_len, unsigned char *out, size_t *out_len) { unsigned long len = in_len; - return gunzip(out, *out_len, (unsigned char *)in, &len); + return zunzip(out, *out_len, (unsigned char *)in, &len, 0, 0); }
/* Fake description object for the "none" compressor */ diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 91351de..a68e4c1 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -2172,6 +2172,4 @@ int ubifs_decompress(const void *buf, int len, void *out, int *out_len, /* todo: Move these to a common U-Boot header */ int lzo1x_decompress_safe(const unsigned char *in, size_t in_len, unsigned char *out, size_t *out_len); -int gunzip(void *dst, int dstlen, unsigned char *src, unsigned long *lenp); - #endif /* !__UBIFS_H__ */
Dear Ricardo Ribalda Delgado,
In message 1240831297-15862-3-git-send-email-ricardo.ribalda@uam.es you wrote:
Blocks compressed with zlib dont have the full gzip header.
Without this patch, block compressed with zlib cannot be readed!
Signed-off-by: Ricardo Ribalda Delgado ricardo.ribalda@uam.es
...
#include "ubifs.h" +#include <u-boot/zlib.h>
#if !defined(CONFIG_SYS_64BIT_VSPRINTF) #warning Please define CONFIG_SYS_64BIT_VSPRINTF for correct output! @@ -33,15 +34,17 @@ DECLARE_GLOBAL_DATA_PTR;
/* compress.c */
+int zunzip(void *dst, int dstlen, unsigned char *src, unsigned long *lenp,
int stoponerr, int offset);
The prototype declaration should be in the neader file. Please remove here.
- We need a wrapper for gunzip() because the parameters are
*/
- We need a wrapper for zunzip() because the parameters are
- incompatible with the lzo decompressor.
static int gzip_decompress(const unsigned char *in, size_t in_len, unsigned char *out, size_t *out_len) { unsigned long len = in_len;
- return gunzip(out, *out_len, (unsigned char *)in, &len);
- return zunzip(out, *out_len, (unsigned char *)in, &len, 0, 0);
}
If the only purpose of zunzip() is to be used here, then why do we not make the parameters fit the intended purpose, thus avoiding an additional wrapper?
Best regards,
Wolfgang Denk
On Monday 27 April 2009, Wolfgang Denk wrote:
#include "ubifs.h" +#include <u-boot/zlib.h>
#if !defined(CONFIG_SYS_64BIT_VSPRINTF) #warning Please define CONFIG_SYS_64BIT_VSPRINTF for correct output! @@ -33,15 +34,17 @@ DECLARE_GLOBAL_DATA_PTR;
/* compress.c */
+int zunzip(void *dst, int dstlen, unsigned char *src, unsigned long *lenp, + int stoponerr, int offset);
The prototype declaration should be in the neader file. Please remove here.
Ack.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
On Monday 27 April 2009 08:36:55 Wolfgang Denk wrote:
In message Ricardo Ribalda Delgado wrote:
Blocks compressed with zlib dont have the full gzip header.
Without this patch, block compressed with zlib cannot be readed!
Signed-off-by: Ricardo Ribalda Delgado ricardo.ribalda@uam.es
- We need a wrapper for gunzip() because the parameters are
*/
- We need a wrapper for zunzip() because the parameters are
- incompatible with the lzo decompressor.
static int gzip_decompress(const unsigned char *in, size_t in_len, unsigned char *out, size_t *out_len) { unsigned long len = in_len;
- return gunzip(out, *out_len, (unsigned char *)in, &len);
- return zunzip(out, *out_len, (unsigned char *)in, &len, 0, 0);
}
If the only purpose of zunzip() is to be used here, then why do we not make the parameters fit the intended purpose, thus avoiding an additional wrapper?
not sure i follow ... what do you propose changing ?
easylogo can be changed to use this (a few Blackfin boards) because atm, compression support with that is implemented by running `gzip`. this is because i wasnt able to figure out how to get the decompression routines in u- boot to play with a zlib compressed stream (i.e. no gzip header). -mike
Dear Mike Frysinger,
In message 200904271200.39319.vapier@gentoo.org you wrote:
- We need a wrapper for gunzip() because the parameters are
*/
- We need a wrapper for zunzip() because the parameters are
- incompatible with the lzo decompressor.
static int gzip_decompress(const unsigned char *in, size_t in_len, unsigned char *out, size_t *out_len) { unsigned long len = in_len;
- return gunzip(out, *out_len, (unsigned char *)in, &len);
- return zunzip(out, *out_len, (unsigned char *)in, &len, 0, 0);
}
If the only purpose of zunzip() is to be used here, then why do we not make the parameters fit the intended purpose, thus avoiding an additional wrapper?
not sure i follow ... what do you propose changing ?
The comment above seems to indicate that gzip_decompress() is only (?) needed because the zunzip() parameters are incompatible with those of the lzo decompressor.
So if we create zunzip() just for this purpose, then why not create it in a way that the parameters are compatible with those of the lzo decompressor, so we can omit this whole gzip_decompress() wrapper function?
Best regards,
Wolfgang Denk
On Monday 27 April 2009 15:46:25 Wolfgang Denk wrote:
In message Mike Frysinger wrote:
- We need a wrapper for gunzip() because the parameters are
*/
- We need a wrapper for zunzip() because the parameters are
- incompatible with the lzo decompressor.
static int gzip_decompress(const unsigned char *in, size_t in_len, unsigned char *out, size_t *out_len) { unsigned long len = in_len;
- return gunzip(out, *out_len, (unsigned char *)in, &len);
- return zunzip(out, *out_len, (unsigned char *)in, &len, 0, 0);
}
If the only purpose of zunzip() is to be used here, then why do we not make the parameters fit the intended purpose, thus avoiding an additional wrapper?
not sure i follow ... what do you propose changing ?
The comment above seems to indicate that gzip_decompress() is only (?) needed because the zunzip() parameters are incompatible with those of the lzo decompressor.
So if we create zunzip() just for this purpose, then why not create it in a way that the parameters are compatible with those of the lzo decompressor, so we can omit this whole gzip_decompress() wrapper function?
that should be a follow up change, but the wrapper already exists today regardless of Ricardo's fixes. i dont think his changes should be held up to address that.
that direction should probably cover: - fix gunzip code to use size_t's instead of unsigned long - re-order lzo arguments to match zlib rather than other way around since lzo has very few consumers -mike
Dear Mike Frysinger,
In message 200904271644.33760.vapier@gentoo.org you wrote:
that should be a follow up change, but the wrapper already exists today regardless of Ricardo's fixes. i dont think his changes should be held up to address that.
that direction should probably cover:
- fix gunzip code to use size_t's instead of unsigned long
- re-order lzo arguments to match zlib rather than other way around since
lzo has very few consumers
OK, makes sense to me, too.
Best regards,
Wolfgang Denk
Hello
What is the situation now? We apply this patch and then make lzo parameter compatible? I am a bit lost....
On Tuesday 28 April 2009 03:42:30 Ricardo Ribalda Delgado wrote:
What is the situation now? We apply this patch and then make lzo parameter compatible? I am a bit lost....
i think so ... your latest patch looked OK to me -mike
Hello Wolfgang
If the only purpose of zunzip() is to be used here, then why do we not make the parameters fit the intended purpose, thus avoiding an additional wrapper?
The purpose of zunzip is to use it in more places. Like Mike Frysinger said:
<Mike> this really should be a common function not specific to ubifs as there are many other opportunities for things to be compressed directly with zlib and not through gzip (splash/video images come to mind). </Mike>
Also, in order to be "parameter compatible" with lzo, we should keep this wrapper (am I right Stefan?)
Best regards
Dear Ricardo Ribalda Delgado,
In message aa76a2be0904270925kc67bf77i1b3f53c4afbc394f@mail.gmail.com you wrote:
If the only purpose of zunzip() is to be used here, then why do we not make the parameters fit the intended purpose, thus avoiding an additional wrapper?
The purpose of zunzip is to use it in more places. Like Mike Frysinger said:
<Mike> this really should be a common function not specific to ubifs as there are many other opportunities for things to be compressed directly with zlib and not through gzip (splash/video images come to mind). </Mike>
Also, in order to be "parameter compatible" with lzo, we should keep this wrapper (am I right Stefan?)
But cannot these other pleaces use a parameter-compatible version as well, or even better (because they might then eventually use lzo as well, at least more easily) ?
Best regards,
Wolfgang Denk
Dear Ricardo Ribalda Delgado,
In message 1240831297-15862-2-git-send-email-ricardo.ribalda@uam.es you wrote:
Separate gunzip in
gunzip: Find the end of the header and call zunzip. zunzip: Inflate gunzip block without header.
UBI fs blocks can be compresed in lzo, zlib or no-compression. The current implementation of u-boot supported all the compressions but there was a bug in the implementation of the zlib blocks.
UBIFS's Zlib blocks do not have header but they were compressed using gunzip, a function used to decompress gunzip files/sectors with a header.
This patch adds a new function zunzip that uncompress a zlib block with no header.
Signed-off-by: Ricardo Ribalda Delgado ricardo.ribalda@uam.es
...
- if (r != Z_STREAM_END) {
- if ((r != Z_STREAM_END) && (stoponerr==1)) {
I already asked about this change, which ios unrelated to this patch.
Please explain why this is needed, and why you think this should not be split off into a separate commit?
Best regards,
Wolfgang Denk
Dear Ricardo Ribalda Delgado,
In message 1240831297-15862-1-git-send-email-ricardo.ribalda@uam.es you wrote:
If the memory used to copy the link_make is "dirty" the string wont be ended with NULL, throwing out multiple memory bugs.
What is "link_make" ? Do you mean "link_name" ?
Signed-off-by: Ricardo Ribalda Delgado ricardo.ribalda@uam.es
fs/ubifs/ubifs.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 32f9ff8..427d84a 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -641,6 +641,7 @@ int ubifs_load(char *filename, u32 addr, u32 size) ui = ubifs_inode(inode); if (((inode->i_mode & S_IFMT) == S_IFLNK) && ui->data_len) { memcpy(link_name, ui->data, ui->data_len);
link_name[ui->data_len] = '\0';-- 1.6.2.4
Best regards,
Wolfgang Denk
participants (4)
-
Mike Frysinger -
Ricardo Ribalda Delgado -
Stefan Roese -
Wolfgang Denk