Re: [PATCH v3 12/21] objtool: Refactor elf_add_data() to use a growable data buffer

On Thu, May 14, 2026 at 11:13:55PM +0000, [email protected] wrote:
> > -	sec->data->d_buf = calloc(1, size);
> > -	if (!sec->data->d_buf) {
> > -		ERROR_GLIBC("calloc");
> > -		return NULL;
> > +	size_old = sec->data->d_size;
> > +	offset = ALIGN(size_old, sec->sh.sh_addralign);
> > +	size_new = offset + size;
> 
> Could this ALIGN calculation overflow if a malformed ELF file provides an
> extremely large sh_addralign value?

Doesn't seem realistic...

> > -	if (data)
> > -		memcpy(sec->data->d_buf, data, size);
> > +	if (!sec->data_overallocated)
> > +		alloc_size_old = size_old;
> > +	else
> > +		alloc_size_old = max(64UL, roundup_pow_of_two(size_old ? : 1));
> >  
> > -	sec->data->d_size = size;
> > -	sec->data->d_align = sec->sh.sh_addralign;
> > +	alloc_size_new = max(64UL, roundup_pow_of_two(size_new ? : 1));
> >  
> > -	offset = ALIGN(sec_size(sec), sec->sh.sh_addralign);
> > -	sec->sh.sh_size = offset + size;
> > +	if (alloc_size_new > alloc_size_old) {
> > +		void *orig_buf = sec->data->d_buf;
> > +
> > +		sec->data->d_buf = calloc(1, alloc_size_new);
> > +		if (!sec->data->d_buf) {
> > +			ERROR_GLIBC("calloc");
> > +			return NULL;
> > +		}
> 
> If calloc fails here, does assigning its result directly to sec->data->d_buf
> clobber the valid pointer to orig_buf?
> 
> It appears orig_buf is never freed on this error path, which might leak the
> original buffer if it was previously owned by objtool. Would it be safer to
> keep the original buffer pointer intact on allocation failure?

A memory leak on the error path is fine, it's about to exit anyway.

-- 
Josh