Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 19 Jan 2021 08:25:28 +0200
From:      Toomas Soome <tsoome@me.com>
To:        Ravi Pokala <rpokala@freebsd.org>
Cc:        Toomas Soome <tsoome@FreeBSD.org>, "<src-committers@freebsd.org>" <src-committers@FreeBSD.org>, "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@FreeBSD.org>, "<dev-commits-src-main@freebsd.org>" <dev-commits-src-main@FreeBSD.org>
Subject:   Re: ad1ebbe5cea8 - main - loader: create local copy of mode list provided by vbeinfoblock
Message-ID:  <B62DA04E-15CB-4190-A7F7-23008D54A2BE@me.com>
In-Reply-To: <7FCB573F-0BC6-429E-919C-6A87513DBD63@panasas.com>
References:  <202101160823.10G8NjEi016925@gitrepo.freebsd.org> <7FCB573F-0BC6-429E-919C-6A87513DBD63@panasas.com>

next in thread | previous in thread | raw e-mail | index | archive | help



> On 19. Jan 2021, at 03:24, Ravi Pokala <rpokala@freebsd.org> wrote:
> 
> Hi Toomas,
> 
> -----Original Message-----
> From: <owner-src-committers@freebsd.org> on behalf of Toomas Soome <tsoome@FreeBSD.org>
> Date: 2021-01-16, Saturday at 00:23
> To: <src-committers@FreeBSD.org>, <dev-commits-src-all@FreeBSD.org>, <dev-commits-src-main@FreeBSD.org>
> Subject: git: ad1ebbe5cea8 - main - loader: create local copy of mode list provided by vbeinfoblock
> 
>    The branch main has been updated by tsoome:
> 
>    URL: https://cgit.FreeBSD.org/src/commit/?id=ad1ebbe5cea8ffac0037966990ddf0f80faa55d5
> 
>    commit ad1ebbe5cea8ffac0037966990ddf0f80faa55d5
>    Author:     Toomas Soome <tsoome@FreeBSD.org>
>    AuthorDate: 2021-01-16 10:18:32 +0000
>    Commit:     Toomas Soome <tsoome@FreeBSD.org>
>    CommitDate: 2021-01-16 10:23:22 +0000
> 
>        loader: create local copy of mode list provided by vbeinfoblock
> 
>        Apparently some systems do corrupt mode list memory area, so we need
>        to use local copy instead.
> 
> ...
> 
>    +	vbe_mode_list_size = (uintptr_t)p - (uintptr_t)ml;
>    +	vbe_mode_list = malloc(vbe_mode_list_size);
>    +	if (vbe_mode_list == NULL) {
>    +		free(vbe);
>    +		vbe = NULL;
>    +		free(vbe_mode);
>    +		vbe_mode = NULL;
>    +	}
>    +	bcopy(ml, vbe_mode_list, vbe_mode_list_size);
>    +
>    +	/* reset VideoModePtr, so we will not have chance to use bad data. */
>    +	vbe->VideoModePtr = 0;
> 
> If allocation of vbe_mode_list failed, you're freeing things that were allocated before it, but then you're continuing on. The very next thing you do is the bcopy(), which dereferences vbe_mode_list, which is NULL because of the allocation failure.
> 
> That doesn't seem right.
> 
> Thanks,
> 
> Ravi (rpokala@)
> 
> 


yes, thanks! I do have fix waiting for any additional comments (Yuri Pankov was very kind to provide review feedback). So the update will be posted soon.

thanks,
toomas




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B62DA04E-15CB-4190-A7F7-23008D54A2BE>