Message ID | 1499068136-10597-1-git-send-email-ivan.hu@canonical.com |
---|---|
State | Accepted |
Headers | show |
On 03/07/17 08:48, Ivan Hu wrote: > From: Geliang Tang <geliangtang@gmail.com> > > Sync up with kernel driver efi_test: > > efi/efi_test: Use memdup_user() helper > > Use memdup_user() helper instead of open-coding to simplify the code. > > Signed-off-by: Geliang Tang <geliangtang@gmail.com> > Signed-off-by: Ivan Hu <ivan.hu@canonical.com> > --- > efi_runtime/efi_runtime.c | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c > index 19b624c..6570a54 100644 > --- a/efi_runtime/efi_runtime.c > +++ b/efi_runtime/efi_runtime.c > @@ -90,18 +90,13 @@ copy_ucs2_from_user_len(efi_char16_t **dst, efi_char16_t __user *src, > if (!access_ok(VERIFY_READ, src, 1)) > return -EFAULT; > > - buf = kmalloc(len, GFP_KERNEL); > - if (!buf) { > + buf = memdup_user(src, len); > + if (IS_ERR(buf)) { > *dst = NULL; > - return -ENOMEM; > + return PTR_ERR(buf); > } > *dst = buf; > > - if (copy_from_user(*dst, src, len)) { > - kfree(buf); > - return -EFAULT; > - } > - > return 0; > } > > Ivan, this is great for the latest kernel, what do we do for backwards compatibility in the fwts DKMS driver? Acked-by: Colin Ian King <colin.king@canonical.com>
On 07/03/2017 03:54 PM, Colin Ian King wrote: > On 03/07/17 08:48, Ivan Hu wrote: >> From: Geliang Tang <geliangtang@gmail.com> >> >> Sync up with kernel driver efi_test: >> >> efi/efi_test: Use memdup_user() helper >> >> Use memdup_user() helper instead of open-coding to simplify the code. >> >> Signed-off-by: Geliang Tang <geliangtang@gmail.com> >> Signed-off-by: Ivan Hu <ivan.hu@canonical.com> >> --- >> efi_runtime/efi_runtime.c | 11 +++-------- >> 1 file changed, 3 insertions(+), 8 deletions(-) >> >> diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c >> index 19b624c..6570a54 100644 >> --- a/efi_runtime/efi_runtime.c >> +++ b/efi_runtime/efi_runtime.c >> @@ -90,18 +90,13 @@ copy_ucs2_from_user_len(efi_char16_t **dst, efi_char16_t __user *src, >> if (!access_ok(VERIFY_READ, src, 1)) >> return -EFAULT; >> >> - buf = kmalloc(len, GFP_KERNEL); >> - if (!buf) { >> + buf = memdup_user(src, len); >> + if (IS_ERR(buf)) { >> *dst = NULL; >> - return -ENOMEM; >> + return PTR_ERR(buf); >> } >> *dst = buf; >> >> - if (copy_from_user(*dst, src, len)) { >> - kfree(buf); >> - return -EFAULT; >> - } >> - >> return 0; >> } >> >> > Ivan, this is great for the latest kernel, what do we do for backwards > compatibility in the fwts DKMS driver? > Colin, memdup_user has introduced for a very long time,since 2009. Looks like should not be a problem for the DKMS driver. What backward compatibility issue have you seen? Ivan > Acked-by: Colin Ian King <colin.king@canonical.com> >
On 03/07/17 09:14, ivanhu wrote: > > > On 07/03/2017 03:54 PM, Colin Ian King wrote: >> On 03/07/17 08:48, Ivan Hu wrote: >>> From: Geliang Tang <geliangtang@gmail.com> >>> >>> Sync up with kernel driver efi_test: >>> >>> efi/efi_test: Use memdup_user() helper >>> >>> Use memdup_user() helper instead of open-coding to simplify the code. >>> >>> Signed-off-by: Geliang Tang <geliangtang@gmail.com> >>> Signed-off-by: Ivan Hu <ivan.hu@canonical.com> >>> --- >>> efi_runtime/efi_runtime.c | 11 +++-------- >>> 1 file changed, 3 insertions(+), 8 deletions(-) >>> >>> diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c >>> index 19b624c..6570a54 100644 >>> --- a/efi_runtime/efi_runtime.c >>> +++ b/efi_runtime/efi_runtime.c >>> @@ -90,18 +90,13 @@ copy_ucs2_from_user_len(efi_char16_t **dst, >>> efi_char16_t __user *src, >>> if (!access_ok(VERIFY_READ, src, 1)) >>> return -EFAULT; >>> - buf = kmalloc(len, GFP_KERNEL); >>> - if (!buf) { >>> + buf = memdup_user(src, len); >>> + if (IS_ERR(buf)) { >>> *dst = NULL; >>> - return -ENOMEM; >>> + return PTR_ERR(buf); >>> } >>> *dst = buf; >>> - if (copy_from_user(*dst, src, len)) { >>> - kfree(buf); >>> - return -EFAULT; >>> - } >>> - >>> return 0; >>> } >>> >> Ivan, this is great for the latest kernel, what do we do for backwards >> compatibility in the fwts DKMS driver? >> > > Colin, memdup_user has introduced for a very long time,since 2009. Looks > like should not be a problem for the DKMS driver. What backward > compatibility issue have you seen? Ah, I hadn't realized that memdep_user was quite that old. No problem then. Sorry for the noise. Colin > > Ivan > >> Acked-by: Colin Ian King <colin.king@canonical.com> >>
On 2017-07-03 12:48 AM, Ivan Hu wrote: > From: Geliang Tang <geliangtang@gmail.com> > > Sync up with kernel driver efi_test: > > efi/efi_test: Use memdup_user() helper > > Use memdup_user() helper instead of open-coding to simplify the code. > > Signed-off-by: Geliang Tang <geliangtang@gmail.com> > Signed-off-by: Ivan Hu <ivan.hu@canonical.com> > --- > efi_runtime/efi_runtime.c | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c > index 19b624c..6570a54 100644 > --- a/efi_runtime/efi_runtime.c > +++ b/efi_runtime/efi_runtime.c > @@ -90,18 +90,13 @@ copy_ucs2_from_user_len(efi_char16_t **dst, efi_char16_t __user *src, > if (!access_ok(VERIFY_READ, src, 1)) > return -EFAULT; > > - buf = kmalloc(len, GFP_KERNEL); > - if (!buf) { > + buf = memdup_user(src, len); > + if (IS_ERR(buf)) { > *dst = NULL; > - return -ENOMEM; > + return PTR_ERR(buf); > } > *dst = buf; > > - if (copy_from_user(*dst, src, len)) { > - kfree(buf); > - return -EFAULT; > - } > - > return 0; > } > > Acked-by: Alex Hung <alex.hung@canonical.com>
diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c index 19b624c..6570a54 100644 --- a/efi_runtime/efi_runtime.c +++ b/efi_runtime/efi_runtime.c @@ -90,18 +90,13 @@ copy_ucs2_from_user_len(efi_char16_t **dst, efi_char16_t __user *src, if (!access_ok(VERIFY_READ, src, 1)) return -EFAULT; - buf = kmalloc(len, GFP_KERNEL); - if (!buf) { + buf = memdup_user(src, len); + if (IS_ERR(buf)) { *dst = NULL; - return -ENOMEM; + return PTR_ERR(buf); } *dst = buf; - if (copy_from_user(*dst, src, len)) { - kfree(buf); - return -EFAULT; - } - return 0; }