Module.execute does not check input count

[GregoryComer]

🐛 Describe the bug

The execute function in module.cpp will cause silent memory corruption if too many input EValues are passed.

executorch/extension/module/module.cpp

Lines 233 to 244 in 976fe48

	runtime::Result<std::vector<runtime::EValue>> Module::execute(
	const std::string& method_name,
	const std::vector<runtime::EValue>& input_values) {
	ET_CHECK_OK_OR_RETURN_ERROR(load_method(method_name));
	auto& method = methods_.at(method_name).method;
	auto& inputs = methods_.at(method_name).inputs;
	for (size_t i = 0; i < input_values.size(); ++i) {
	if (!input_values[i].isNone()) {
	inputs[i] = input_values[i];
	}
	}

If input_values.size() > inputs.size(), the indexing operation on line 242 will write past the end of the array. I encountered this when I updated a model definition but hadn't updated the C++ runner code yet and it caused a memory-related crash when destructing the module.

We should check for input_values.size() <= inputs.size() and log an error + return an error code (Error::InvalidArgument) if the check fails.

As an aside, passing fewer inputs in may or may not be a desirable feature, given that some may be memory planned or unchanged between calls. We can ignore this case for the sake of this bug as it's not a correctness issue, but it might be good to error out for clarity if too few inputs are passed.

Versions

Repro-able on main, though it's likely been like this for a long time (or forever).

[keyprocedure]

I'd be happy to work on this!

[GregoryComer]

I'd be happy to work on this!

Thanks, that would be great, if you're interested. I'll tentatively mark you as the owner of this task.