Syslinux - Aug 2012 - [PATCH] add additional checks to ext2 loader

Check if some pointers are not NULL due to read errors or other problems

Signed-off-by: Frediano Ziglio <frediano.ziglio at citrix.com>
---
 core/fs/ext2/ext2.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/core/fs/ext2/ext2.c b/core/fs/ext2/ext2.c
index bddde8d..8f0f2a4 100644
--- a/core/fs/ext2/ext2.c
+++ b/core/fs/ext2/ext2.c
@@ -139,6 +139,8 @@ ext2_get_inode(struct fs_info *fs, int inr)
     block_off = inode_offset % EXT2_INODES_PER_BLOCK(fs);
 
     data = get_cache(fs->fs_dev, block_num);
+    if (!data)
+	return NULL;
 
     return (const struct ext2_inode *)
 	(data + block_off * EXT2_SB(fs)->s_inode_size);
@@ -164,7 +166,7 @@ static struct inode *ext2_iget_by_inr(struct fs_info *fs,
uint32_t inr)
     struct inode *inode;
 
     e_inode = ext2_get_inode(fs, inr);
-    if (!(inode = alloc_inode(fs, inr, sizeof(struct ext2_pvt_inode))))
+    if (!e_inode || !(inode = alloc_inode(fs, inr, sizeof(struct
ext2_pvt_inode))))
 	return NULL;
     fill_inode(inode, e_inode);
 
-- 
1.7.5.4

On Thu, 2012-08-02 at 09:32 +0100, Frediano Ziglio
wrote:
> Check if some pointers are not NULL due to read errors or other problems
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio at citrix.com>
> ---
>  core/fs/ext2/ext2.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/core/fs/ext2/ext2.c b/core/fs/ext2/ext2.c
> index bddde8d..8f0f2a4 100644
> --- a/core/fs/ext2/ext2.c
> +++ b/core/fs/ext2/ext2.c
> @@ -139,6 +139,8 @@ ext2_get_inode(struct fs_info *fs, int inr)
>      block_off = inode_offset % EXT2_INODES_PER_BLOCK(fs);
>  
>      data = get_cache(fs->fs_dev, block_num);
> +    if (!data)
> +	return NULL;
>  
>      return (const struct ext2_inode *)
>  	(data + block_off * EXT2_SB(fs)->s_inode_size);
I'm sure Peter will correct me I'm wrong here, but I'm fairly
certain
that get_cache() cannot return NULL. Look at it's implementation to see
what I mean.
> @@ -164,7 +166,7 @@ static struct inode *ext2_iget_by_inr(struct fs_info
*fs, uint32_t inr)
>      struct inode *inode;
>  
>      e_inode = ext2_get_inode(fs, inr);
> -    if (!(inode = alloc_inode(fs, inr, sizeof(struct ext2_pvt_inode))))
> +    if (!e_inode || !(inode = alloc_inode(fs, inr, sizeof(struct
ext2_pvt_inode))))
>  	return NULL;
>      fill_inode(inode, e_inode);
>  
On the other hand, this check *is* needed, so I've applied this last
snippet.

-- 
Matt Fleming, Intel Open Source Technology Center

From: Frediano Ziglio <frediano.ziglio at citrix.com>
Date: Thu, 2 Aug 2012 09:32:36 +0100
>      e_inode = ext2_get_inode(fs, inr);
> -    if (!(inode = alloc_inode(fs, inr, sizeof(struct ext2_pvt_inode))))
> +    if (!e_inode || !(inode = alloc_inode(fs, inr, sizeof(struct
ext2_pvt_inode))))
>  	return NULL;
You could at least refactor this. This if-statement is just horrible and hard to
read at glance.

		Paulo