On 6/14/25 11:28 PM, Daniel Gomez wrote:
>> This seems to be off by one. For instance, if the loop reaches the last
>> valid type in mod_mem_type, MOD_INIT_RODATA, and successfully allocates
>> its memory, the variable t gets set to MOD_INIT_RODATA. Subsequently, if
>> an error occurs later in move_module() and control is transferred to
>> out_err, the deallocation starts from t-1, and therefore MOD_INIT_RODATA
>> doesn't get freed.
>>
>> If we want to always start from the last type found, the code would need
>> to be:
>>
>>              [...]
>>              ret = module_memory_alloc(mod, type);
>>              if (ret)
>>                      goto out_err;
>>              t = type + 1;
>>      }
>>
>> I can adjust it in this way if it is preferred.
>>
> 
> My earlier suggestion was incorrect. We can simply initialize the memory
> type t to MOD_MEM_NUM_TYPES since it's only used in the error path of
> module_memory_alloc().

Do you mean the following, or something else:

static int move_module(struct module *mod, struct load_info *info)
{
        int i;
        enum mod_mem_type t = MOD_MEM_NUM_TYPES;
        int ret;
        bool codetag_section_found = false;

        for_each_mod_mem_type(type) {
                if (!mod->mem[type].size) {
                        mod->mem[type].base = NULL;
                        continue;
                }

                ret = module_memory_alloc(mod, type);
                if (ret) {
                        t = type;
                        goto out_err;
                }
        }

        [...]
}

-- 
Thanks,
Petr

Reply via email to