On Tue, 8 Nov 2022 12:38:53 -0800 Yonghong Song <yhs@xxxxxxxx> wrote: > On 11/8/22 12:35 PM, Yonghong Song wrote: > > > > > > On 11/8/22 11:52 AM, Francis Laniel wrote: > >> From: Alban Crequy <albancrequy@xxxxxxxxxxxxx> > >> > >> If a page fault occurs while copying the first byte, this function > >> resets one > >> byte before dst. > >> As a consequence, an address could be modified and leaded to > >> kernel crashes if > >> case the modified address was accessed later. > >> > >> Signed-off-by: Alban Crequy <albancrequy@xxxxxxxxxxxxx> > >> Tested-by: Francis Laniel <flaniel@xxxxxxxxxxxxxxxxxxx> > >> --- > >> mm/maccess.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/mm/maccess.c b/mm/maccess.c > >> index 5f4d240f67ec..074f6b086671 100644 > >> --- a/mm/maccess.c > >> +++ b/mm/maccess.c > >> @@ -97,7 +97,7 @@ long strncpy_from_kernel_nofault(char *dst, > >> const void *unsafe_addr, long count) > >> return src - unsafe_addr; > >> Efault: > >> pagefault_enable(); > >> - dst[-1] = '\0'; > >> + dst[0] = '\0'; > > > > What if the fault is due to dst, so the above won't work, right? > > > > The original code should work fine if the first byte copy is > > successful. For the first byte copy fault, maybe just to leave it > > as is? > > Okay, the dst is always safe (from func signature), so change looks > okay to me. Probably mm people can double check. My understanding was that the bpf verifier is supposed to check that the dst pointer is safe. But I don't know where it is done, and I don't know how it can check that the dst buffer is big enough. > > > >> return -EFAULT; > >> } > >> > >> -- > >> 2.25.1 > >> >
