On 5/24/2024 1:21 PM, Greg KH wrote:
On Fri, May 24, 2024 at 01:15:01PM +0800, quic_zijuhu wrote:
On 5/24/2024 12:33 PM, Greg KH wrote:
On Fri, May 24, 2024 at 12:20:03PM +0800, Zijun Hu wrote:
zap_modalias_env() wrongly calculates size of memory block to move, so maybe cause OOB memory access issue, fixed by correcting size to memmove.
"maybe" or "does"? That's a big difference :)
i found this issue by reading code instead of really meeting this issue. this issue should be prone to happen if there are more than 1 other environment vars.
But does it? Given that we have loads of memory checkers, and I haven't ever seen any report of any overrun, it would be nice to be sure.
yes. if @env includes env vairable MODALIAS and more than one other env vairables. then (env->buflen - len) must be greater that actual size of "target block" shown previously, so the OOB issue must happen.
do you have suggestion about term to use?
Some confirmation that this really is the case would be nice :)
Fixes: 9b3fa47d4a76 ("kobject: fix suppressing modalias in uevents delivered over netlink") Cc: stable@vger.kernel.org Signed-off-by: Zijun Hu quic_zijuhu@quicinc.com
lib/kobject_uevent.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c index 03b427e2707e..f153b4f9d4d9 100644 --- a/lib/kobject_uevent.c +++ b/lib/kobject_uevent.c @@ -434,7 +434,7 @@ static void zap_modalias_env(struct kobj_uevent_env *env) if (i != env->envp_idx - 1) { memmove(env->envp[i], env->envp[i + 1],
env->buflen - len);
env->buf + env->buflen - env->envp[i + 1]);How is this "more correct"? Please explain it better, this logic is not obvious at all.
env->envp[] contains pointers to env->buf[] with length env->buflen, we want to delete environment variable pointed by env->envp[i] with length @len as shown below.
env->buf[] |-> target block <-| 0-----------------------------------------env->buflen ^ ^ | -> @len <- | env->envp[i] env->envp[i+1]
so move "target block" forward by @len, so size of target block is env->buf + env->buflen - env->envp[i+1] instead of env->buflen -len.
do you suggest add inline comments to explain it ?
Yes please.
thanks,
greg k-h