Re: [PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
On Thu, Nov 25, 2021 at 06:18:22PM +0300, Dan Carpenter wrote:
> I had thought that ->kmap_cnt was a regular int and not an unsigned
> int, but I would have to pull a stable tree to see where I misread the
> code.
I was looking at (struct ion_buffer)->kmap_cnt but this is
(struct ion_handle)->kmap_cnt. I'm not sure how those are related but
it makes me nervous that one can go higher than the other. Also both
probably need overflow protection.
So I guess I would just do something like:
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 806e9b30b9dc8..e8846279b33b5 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -489,6 +489,8 @@ static void *ion_buffer_kmap_get(struct ion_buffer *buffer)
void *vaddr;
if (buffer->kmap_cnt) {
+ if (buffer->kmap_cnt == INT_MAX)
+ return ERR_PTR(-EOVERFLOW);
buffer->kmap_cnt++;
return buffer->vaddr;
}
@@ -509,6 +511,8 @@ static void *ion_handle_kmap_get(struct ion_handle *handle)
void *vaddr;
if (handle->kmap_cnt) {
+ if (handle->kmap_cnt == INT_MAX)
+ return ERR_PTR(-EOVERFLOW);
handle->kmap_cnt++;
return buffer->vaddr;
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[staging:staging-testing] BUILD SUCCESS 5f154cad675234929957a454dfc55164b6aa4d12
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git staging-testing branch HEAD: 5f154cad675234929957a454dfc55164b6aa4d12 staging: r8188eu: use a delayed worker for led updates elapsed time: 728m configs tested: 54 configs skipped: 3 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig i386 randconfig-c001-20211125 ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig s390 allmodconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig i386 debian-10.3 mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig um x86_64_defconfig um i386_defconfig x86_64 allyesconfig x86_64rhel-8.3-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 rhel-8.3-func x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
On Thu, 25 Nov 2021, Dan Carpenter wrote:
> On Thu, Nov 25, 2021 at 03:07:37PM +, Lee Jones wrote:
> > On Thu, 25 Nov 2021, Dan Carpenter wrote:
> >
> > > On Thu, Nov 25, 2021 at 02:20:04PM +, Lee Jones wrote:
> > > > Supply additional checks in order to prevent unexpected results.
> > > >
> > > > Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
> > > > Signed-off-by: Lee Jones
> > > > ---
> > > > Should be back-ported from v4.9 and earlier.
> > > >
> > > > drivers/staging/android/ion/ion.c | 5 +
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/staging/android/ion/ion.c
> > > > b/drivers/staging/android/ion/ion.c
> > > > index 806e9b30b9dc8..402b74f5d7e69 100644
> > > > --- a/drivers/staging/android/ion/ion.c
> > > > +++ b/drivers/staging/android/ion/ion.c
> > > > @@ -29,6 +29,7 @@
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > #include
> > > > #include
> > > > #include
> > > > @@ -509,6 +510,10 @@ static void *ion_handle_kmap_get(struct ion_handle
> > > > *handle)
> > > > void *vaddr;
> > > >
> > > > if (handle->kmap_cnt) {
> > > > + if (check_add_overflow(handle->kmap_cnt,
> > > > + (unsigned int) 1,
> > > > >kmap_cnt))
> > > ^
> > >
> > > > + return ERR_PTR(-EOVERFLOW);
> > > > +
> > > > handle->kmap_cnt++;
> > > ^^^
> > > This will not do what you want at all. It's a double increment on the
> > > success path and it leave handle->kmap_cnt overflowed on failure path.
> >
> > I read the helper to take copies of the original variables.
> >
> > #define __unsigned_add_overflow(a, b, d) ({ \
> > typeof(a) __a = (a);\
> > typeof(b) __b = (b);\
> > typeof(d) __d = (d);\
> > (void) (&__a == &__b); \
> > (void) (&__a == __d); \
> > *__d = __a + __b; \
>
> This assignment sets handle->kmap_cnt to the overflowed value.
>
> > *__d < __a; \
> > })
> >
> > Maybe I misread it.
> >
> > So the original patch [0] was better?
> >
> > [0]
> > https://lore.kernel.org/stable/20211125120234.67987-1-lee.jo...@linaro.org/
>
> The original at least worked. :P
>
> You're catching me right as I'm knocking off for the day so I'm not
> sure how to write this code. I had thought that ->kmap_cnt was a
> regular int and not an unsigned int, but I would have to pull a stable
> tree to see where I misread the code.
>
> I'll look at this tomorrow Nairobi time, but I expect by then you'll
> already have it all figured out.
There's no rush. This has been broken for a long time.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
On Thu, Nov 25, 2021 at 03:07:37PM +, Lee Jones wrote:
> On Thu, 25 Nov 2021, Dan Carpenter wrote:
>
> > On Thu, Nov 25, 2021 at 02:20:04PM +, Lee Jones wrote:
> > > Supply additional checks in order to prevent unexpected results.
> > >
> > > Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
> > > Signed-off-by: Lee Jones
> > > ---
> > > Should be back-ported from v4.9 and earlier.
> > >
> > > drivers/staging/android/ion/ion.c | 5 +
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/staging/android/ion/ion.c
> > > b/drivers/staging/android/ion/ion.c
> > > index 806e9b30b9dc8..402b74f5d7e69 100644
> > > --- a/drivers/staging/android/ion/ion.c
> > > +++ b/drivers/staging/android/ion/ion.c
> > > @@ -29,6 +29,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -509,6 +510,10 @@ static void *ion_handle_kmap_get(struct ion_handle
> > > *handle)
> > > void *vaddr;
> > >
> > > if (handle->kmap_cnt) {
> > > + if (check_add_overflow(handle->kmap_cnt,
> > > +(unsigned int) 1, >kmap_cnt))
> > ^
> >
> > > + return ERR_PTR(-EOVERFLOW);
> > > +
> > > handle->kmap_cnt++;
> > ^^^
> > This will not do what you want at all. It's a double increment on the
> > success path and it leave handle->kmap_cnt overflowed on failure path.
>
> I read the helper to take copies of the original variables.
>
> #define __unsigned_add_overflow(a, b, d) ({ \
> typeof(a) __a = (a);\
> typeof(b) __b = (b);\
> typeof(d) __d = (d);\
> (void) (&__a == &__b); \
> (void) (&__a == __d); \
> *__d = __a + __b; \
This assignment sets handle->kmap_cnt to the overflowed value.
> *__d < __a; \
> })
>
> Maybe I misread it.
>
> So the original patch [0] was better?
>
> [0]
> https://lore.kernel.org/stable/20211125120234.67987-1-lee.jo...@linaro.org/
The original at least worked. :P
You're catching me right as I'm knocking off for the day so I'm not
sure how to write this code. I had thought that ->kmap_cnt was a
regular int and not an unsigned int, but I would have to pull a stable
tree to see where I misread the code.
I'll look at this tomorrow Nairobi time, but I expect by then you'll
already have it all figured out.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
On Thu, 25 Nov 2021, Lee Jones wrote:
> On Thu, 25 Nov 2021, Dan Carpenter wrote:
>
> > On Thu, Nov 25, 2021 at 02:20:04PM +, Lee Jones wrote:
> > > Supply additional checks in order to prevent unexpected results.
> > >
> > > Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
> > > Signed-off-by: Lee Jones
> > > ---
> > > Should be back-ported from v4.9 and earlier.
> > >
> > > drivers/staging/android/ion/ion.c | 5 +
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/staging/android/ion/ion.c
> > > b/drivers/staging/android/ion/ion.c
> > > index 806e9b30b9dc8..402b74f5d7e69 100644
> > > --- a/drivers/staging/android/ion/ion.c
> > > +++ b/drivers/staging/android/ion/ion.c
> > > @@ -29,6 +29,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -509,6 +510,10 @@ static void *ion_handle_kmap_get(struct ion_handle
> > > *handle)
> > > void *vaddr;
> > >
> > > if (handle->kmap_cnt) {
> > > + if (check_add_overflow(handle->kmap_cnt,
> > > +(unsigned int) 1, >kmap_cnt))
> > ^
> >
> > > + return ERR_PTR(-EOVERFLOW);
> > > +
> > > handle->kmap_cnt++;
> > ^^^
> > This will not do what you want at all. It's a double increment on the
> > success path and it leave handle->kmap_cnt overflowed on failure path.
>
> I read the helper to take copies of the original variables.
>
> #define __unsigned_add_overflow(a, b, d) ({ \
> typeof(a) __a = (a);\
> typeof(b) __b = (b);\
> typeof(d) __d = (d);\
> (void) (&__a == &__b); \
> (void) (&__a == __d); \
> *__d = __a + __b; \
> *__d < __a; \
> })
>
> Maybe I misread it.
I think I see now.
Copies are taken, but because 'd' is a pointer, dereferencing the copy
is just like dereferencing the original, thus the memory address
provided by 'd' is written to, updating the variable.
In this case, you're right, this is not what I was trying to achieve.
> So the original patch [0] was better?
>
> [0]
> https://lore.kernel.org/stable/20211125120234.67987-1-lee.jo...@linaro.org/
Greg, are you able to take the original patch for v4.4 and v4.9 please?
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Your Approved Payment !
THE WORLDS LOCAL BANK International Banking FOREIGN EXCHANGE UNIT RE: MANDATORY RELEASE ORDER OF YOUR OVERDUE FUND Dear Valued Beneficiary: We are pleased to inform you that we have finally concluded arrangement towards your refund/lottery pay out which has been delayed for a Long Period of time because of your Cooperation and Dealings with Wrong Officials and importers of banks as your fund returned back to us on the 4th of Jan 2021 when we confirmed the rate of delays and questionable activities that has been related by the previous administrative banks alongside with others that collaborated in delaying the release of your fund after all charges and payments demanded were paid. Recently, the Ministry of Finance of United Kingdom, Bank of England, HSBC Bank Plc UK and United Kingdom Inland Revenue Services held a meeting on how this fund will be released to the beneficiaries to their designated bank accounts in their country without further delay since we are in the first half of the economic year 2021 and it is now overdue to be released as the said funds belongs to them. We apologize for the delay of the payment and all the inconveniences that this might have caused you during this period of time. However we have instructed all the banks in the globe which we previously asked to help us pay out this fund to the general public to STOP the process of the release of the fund due to their incompetence and negligence of duty towards the release of this fund. After our findings, some were arrested and charged for theft according to Section 1 of the Theft Act 1978, as amended by the Theft (Amendment) Act 1996 law of the United Kingdom. The Bank of England Governor (Mr Andrew Bailey) has given serious warning and Instructions and ordered the Inland Revenue Services Department of England to quickly release all on hold funds which are in their escrow account to the sole beneficiaries which you are among those who will receive their Inheritance funds. Please contact ONLY the Executive member of the Monetary Policy Committee of South African Reserve Bank (Dr Rashad Cassim) on his email: sarb_bnk...@meta.ua to advise you on how to procure the certificate of claim as the law of South Africa demands that without it there will not be any payment whether pending loan amount, lottery fund, inheritance funds or whatsoever fund locally or internationally perhaps you have not yet received it. Provide below details to Dr Rashad Cassim for his clarification: Full Name... Tel. Address. Amount.. City Country. Copies of documents pertaining to the fund. Best Regards, Mr.James Emmett. Chief Executive Officer, HSBC Bank plc. United Kingdom ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
On Thu, 25 Nov 2021, Dan Carpenter wrote:
> On Thu, Nov 25, 2021 at 02:20:04PM +, Lee Jones wrote:
> > Supply additional checks in order to prevent unexpected results.
> >
> > Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
> > Signed-off-by: Lee Jones
> > ---
> > Should be back-ported from v4.9 and earlier.
> >
> > drivers/staging/android/ion/ion.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/staging/android/ion/ion.c
> > b/drivers/staging/android/ion/ion.c
> > index 806e9b30b9dc8..402b74f5d7e69 100644
> > --- a/drivers/staging/android/ion/ion.c
> > +++ b/drivers/staging/android/ion/ion.c
> > @@ -29,6 +29,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -509,6 +510,10 @@ static void *ion_handle_kmap_get(struct ion_handle
> > *handle)
> > void *vaddr;
> >
> > if (handle->kmap_cnt) {
> > + if (check_add_overflow(handle->kmap_cnt,
> > + (unsigned int) 1, >kmap_cnt))
> ^
>
> > + return ERR_PTR(-EOVERFLOW);
> > +
> > handle->kmap_cnt++;
> ^^^
> This will not do what you want at all. It's a double increment on the
> success path and it leave handle->kmap_cnt overflowed on failure path.
I read the helper to take copies of the original variables.
#define __unsigned_add_overflow(a, b, d) ({ \
typeof(a) __a = (a);\
typeof(b) __b = (b);\
typeof(d) __d = (d);\
(void) (&__a == &__b); \
(void) (&__a == __d); \
*__d = __a + __b; \
*__d < __a; \
})
Maybe I misread it.
So the original patch [0] was better?
[0] https://lore.kernel.org/stable/20211125120234.67987-1-lee.jo...@linaro.org/
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
On Thu, Nov 25, 2021 at 02:20:04PM +, Lee Jones wrote:
> Supply additional checks in order to prevent unexpected results.
>
> Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
> Signed-off-by: Lee Jones
> ---
> Should be back-ported from v4.9 and earlier.
>
> drivers/staging/android/ion/ion.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/staging/android/ion/ion.c
> b/drivers/staging/android/ion/ion.c
> index 806e9b30b9dc8..402b74f5d7e69 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -509,6 +510,10 @@ static void *ion_handle_kmap_get(struct ion_handle
> *handle)
> void *vaddr;
>
> if (handle->kmap_cnt) {
> + if (check_add_overflow(handle->kmap_cnt,
> +(unsigned int) 1, >kmap_cnt))
^
> + return ERR_PTR(-EOVERFLOW);
> +
> handle->kmap_cnt++;
^^^
This will not do what you want at all. It's a double increment on the
success path and it leave handle->kmap_cnt overflowed on failure path.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
Supply additional checks in order to prevent unexpected results.
Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
Signed-off-by: Lee Jones
---
Should be back-ported from v4.9 and earlier.
drivers/staging/android/ion/ion.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 806e9b30b9dc8..402b74f5d7e69 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -509,6 +510,10 @@ static void *ion_handle_kmap_get(struct ion_handle *handle)
void *vaddr;
if (handle->kmap_cnt) {
+ if (check_add_overflow(handle->kmap_cnt,
+ (unsigned int) 1, >kmap_cnt))
+ return ERR_PTR(-EOVERFLOW);
+
handle->kmap_cnt++;
return buffer->vaddr;
}
--
2.34.0.rc2.393.gf8c9666880-goog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
On Thu, 25 Nov 2021, Greg KH wrote:
> On Thu, Nov 25, 2021 at 01:03:46PM +, Lee Jones wrote:
> > On Thu, 25 Nov 2021, Greg KH wrote:
> >
> > > On Thu, Nov 25, 2021 at 12:46:23PM +, Lee Jones wrote:
> > > > On Thu, 25 Nov 2021, Greg KH wrote:
> > > >
> > > > > On Thu, Nov 25, 2021 at 12:02:34PM +, Lee Jones wrote:
> > > > > > Supply additional checks in order to prevent unexpected results.
> > > > > >
> > > > > > Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
> > > > > > Signed-off-by: Lee Jones
> > > > > > ---
> > > > > > drivers/staging/android/ion/ion.c | 3 +++
> > > > > > 1 file changed, 3 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/staging/android/ion/ion.c
> > > > > > b/drivers/staging/android/ion/ion.c
> > > > > > index 806e9b30b9dc8..30f359faba575 100644
> > > > > > --- a/drivers/staging/android/ion/ion.c
> > > > > > +++ b/drivers/staging/android/ion/ion.c
> > > > > > @@ -509,6 +509,9 @@ static void *ion_handle_kmap_get(struct
> > > > > > ion_handle *handle)
> > > > > > void *vaddr;
> > > > > >
> > > > > > if (handle->kmap_cnt) {
> > > > > > + if (handle->kmap_cnt + 1 < handle->kmap_cnt)
> > > > >
> > > > > What about using the nice helpers in overflow.h for this?
> > > >
> > > > I haven't heard of these before.
> > > >
> > > > Looks like they're not widely used.
> > > >
> > > > I'll try them out and see how they go.
> > > >
> > > > > > + return ERR_PTR(-EOVERFLOW);
> > > > > > +
> > > > > > handle->kmap_cnt++;
> > > > > > return buffer->vaddr;
> > > > > > }
> > > > >
> > > > > What stable kernel branch(es) is this for?
> > > >
> > > > I assumed your magic scripts could determine this from the Fixes:
> > > > tag. I'll be more explicit in v2.
> > >
> > > The fixes tag says how far back for it to go, but not where to start
> > > that process from :)
> >
> > What's your preferred method for identifying a start-point?
> >
> > In the [PATCH] tag or appended on to Cc: stable ... # ?
> >
> > I know both work, but what makes your life easier?
>
> Easiest is below the --- line say:
> ---
> This is for kernel versions X.X and older.
Understood, thanks.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
On Thu, Nov 25, 2021 at 01:03:46PM +, Lee Jones wrote:
> On Thu, 25 Nov 2021, Greg KH wrote:
>
> > On Thu, Nov 25, 2021 at 12:46:23PM +, Lee Jones wrote:
> > > On Thu, 25 Nov 2021, Greg KH wrote:
> > >
> > > > On Thu, Nov 25, 2021 at 12:02:34PM +, Lee Jones wrote:
> > > > > Supply additional checks in order to prevent unexpected results.
> > > > >
> > > > > Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
> > > > > Signed-off-by: Lee Jones
> > > > > ---
> > > > > drivers/staging/android/ion/ion.c | 3 +++
> > > > > 1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/drivers/staging/android/ion/ion.c
> > > > > b/drivers/staging/android/ion/ion.c
> > > > > index 806e9b30b9dc8..30f359faba575 100644
> > > > > --- a/drivers/staging/android/ion/ion.c
> > > > > +++ b/drivers/staging/android/ion/ion.c
> > > > > @@ -509,6 +509,9 @@ static void *ion_handle_kmap_get(struct
> > > > > ion_handle *handle)
> > > > > void *vaddr;
> > > > >
> > > > > if (handle->kmap_cnt) {
> > > > > + if (handle->kmap_cnt + 1 < handle->kmap_cnt)
> > > >
> > > > What about using the nice helpers in overflow.h for this?
> > >
> > > I haven't heard of these before.
> > >
> > > Looks like they're not widely used.
> > >
> > > I'll try them out and see how they go.
> > >
> > > > > + return ERR_PTR(-EOVERFLOW);
> > > > > +
> > > > > handle->kmap_cnt++;
> > > > > return buffer->vaddr;
> > > > > }
> > > >
> > > > What stable kernel branch(es) is this for?
> > >
> > > I assumed your magic scripts could determine this from the Fixes:
> > > tag. I'll be more explicit in v2.
> >
> > The fixes tag says how far back for it to go, but not where to start
> > that process from :)
>
> What's your preferred method for identifying a start-point?
>
> In the [PATCH] tag or appended on to Cc: stable ... # ?
>
> I know both work, but what makes your life easier?
Easiest is below the --- line say:
---
This is for kernel versions X.X and older.
nothing complex.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
On Thu, 25 Nov 2021, Greg KH wrote:
> On Thu, Nov 25, 2021 at 12:46:23PM +, Lee Jones wrote:
> > On Thu, 25 Nov 2021, Greg KH wrote:
> >
> > > On Thu, Nov 25, 2021 at 12:02:34PM +, Lee Jones wrote:
> > > > Supply additional checks in order to prevent unexpected results.
> > > >
> > > > Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
> > > > Signed-off-by: Lee Jones
> > > > ---
> > > > drivers/staging/android/ion/ion.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/staging/android/ion/ion.c
> > > > b/drivers/staging/android/ion/ion.c
> > > > index 806e9b30b9dc8..30f359faba575 100644
> > > > --- a/drivers/staging/android/ion/ion.c
> > > > +++ b/drivers/staging/android/ion/ion.c
> > > > @@ -509,6 +509,9 @@ static void *ion_handle_kmap_get(struct ion_handle
> > > > *handle)
> > > > void *vaddr;
> > > >
> > > > if (handle->kmap_cnt) {
> > > > + if (handle->kmap_cnt + 1 < handle->kmap_cnt)
> > >
> > > What about using the nice helpers in overflow.h for this?
> >
> > I haven't heard of these before.
> >
> > Looks like they're not widely used.
> >
> > I'll try them out and see how they go.
> >
> > > > + return ERR_PTR(-EOVERFLOW);
> > > > +
> > > > handle->kmap_cnt++;
> > > > return buffer->vaddr;
> > > > }
> > >
> > > What stable kernel branch(es) is this for?
> >
> > I assumed your magic scripts could determine this from the Fixes:
> > tag. I'll be more explicit in v2.
>
> The fixes tag says how far back for it to go, but not where to start
> that process from :)
What's your preferred method for identifying a start-point?
In the [PATCH] tag or appended on to Cc: stable ... # ?
I know both work, but what makes your life easier?
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
On Thu, Nov 25, 2021 at 12:46:23PM +, Lee Jones wrote:
> On Thu, 25 Nov 2021, Greg KH wrote:
>
> > On Thu, Nov 25, 2021 at 12:02:34PM +, Lee Jones wrote:
> > > Supply additional checks in order to prevent unexpected results.
> > >
> > > Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
> > > Signed-off-by: Lee Jones
> > > ---
> > > drivers/staging/android/ion/ion.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/staging/android/ion/ion.c
> > > b/drivers/staging/android/ion/ion.c
> > > index 806e9b30b9dc8..30f359faba575 100644
> > > --- a/drivers/staging/android/ion/ion.c
> > > +++ b/drivers/staging/android/ion/ion.c
> > > @@ -509,6 +509,9 @@ static void *ion_handle_kmap_get(struct ion_handle
> > > *handle)
> > > void *vaddr;
> > >
> > > if (handle->kmap_cnt) {
> > > + if (handle->kmap_cnt + 1 < handle->kmap_cnt)
> >
> > What about using the nice helpers in overflow.h for this?
>
> I haven't heard of these before.
>
> Looks like they're not widely used.
>
> I'll try them out and see how they go.
>
> > > + return ERR_PTR(-EOVERFLOW);
> > > +
> > > handle->kmap_cnt++;
> > > return buffer->vaddr;
> > > }
> >
> > What stable kernel branch(es) is this for?
>
> I assumed your magic scripts could determine this from the Fixes:
> tag. I'll be more explicit in v2.
The fixes tag says how far back for it to go, but not where to start
that process from :)
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
On Thu, 25 Nov 2021, Greg KH wrote:
> On Thu, Nov 25, 2021 at 12:02:34PM +, Lee Jones wrote:
> > Supply additional checks in order to prevent unexpected results.
> >
> > Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
> > Signed-off-by: Lee Jones
> > ---
> > drivers/staging/android/ion/ion.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/staging/android/ion/ion.c
> > b/drivers/staging/android/ion/ion.c
> > index 806e9b30b9dc8..30f359faba575 100644
> > --- a/drivers/staging/android/ion/ion.c
> > +++ b/drivers/staging/android/ion/ion.c
> > @@ -509,6 +509,9 @@ static void *ion_handle_kmap_get(struct ion_handle
> > *handle)
> > void *vaddr;
> >
> > if (handle->kmap_cnt) {
> > + if (handle->kmap_cnt + 1 < handle->kmap_cnt)
>
> What about using the nice helpers in overflow.h for this?
I haven't heard of these before.
Looks like they're not widely used.
I'll try them out and see how they go.
> > + return ERR_PTR(-EOVERFLOW);
> > +
> > handle->kmap_cnt++;
> > return buffer->vaddr;
> > }
>
> What stable kernel branch(es) is this for?
I assumed your magic scripts could determine this from the Fixes:
tag. I'll be more explicit in v2.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] binder: avoid potential data leakage when copying txn
Hi Todd,
I love your patch! Perhaps something to improve:
[auto build test WARNING on staging/staging-testing]
[also build test WARNING on v5.16-rc2 next-20211125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Todd-Kjos/binder-Prevent-untranslated-sender-data-from-being-copied-to-target/20211124-031908
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
1189d2fb15a4b09b2e8dd01d60a0817d985d933d
config: ia64-randconfig-s032-20211123
(https://download.01.org/0day-ci/archive/20211125/202111252042.kcpmelly-...@intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
#
https://github.com/0day-ci/linux/commit/d51c5e7a3791e9e748200416f85456c826d3030e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Todd-Kjos/binder-Prevent-untranslated-sender-data-from-being-copied-to-target/20211124-031908
git checkout d51c5e7a3791e9e748200416f85456c826d3030e
# save the config file to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=ia64
SHELL=/bin/bash drivers/android/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
sparse warnings: (new ones prefixed by >>)
>> drivers/android/binder.c:2707:17: sparse: sparse: cast removes address space
>> '__user' of expression
drivers/android/binder.c:2716:17: sparse: sparse: cast removes address space
'__user' of expression
drivers/android/binder.c:4507:24: sparse: sparse: incorrect type in return
expression (different base types) @@ expected restricted __poll_t @@
got int @@
drivers/android/binder.c:4507:24: sparse: expected restricted __poll_t
drivers/android/binder.c:4507:24: sparse: got int
vim +/__user +2707 drivers/android/binder.c
512cf465ee01eb drivers/android/binder.c Todd Kjos
2017-09-29 2457
355b0502f6efea drivers/staging/android/binder.c Greg Kroah-Hartman
2011-11-30 2458 static void binder_transaction(struct binder_proc *proc,
355b0502f6efea drivers/staging/android/binder.c Greg Kroah-Hartman
2011-11-30 2459 struct binder_thread *thread,
4bfac80af3a63f drivers/android/binder.c Martijn Coenen
2017-02-03 2460 struct binder_transaction_data
*tr, int reply,
4bfac80af3a63f drivers/android/binder.c Martijn Coenen
2017-02-03 2461 binder_size_t
extra_buffers_size)
355b0502f6efea drivers/staging/android/binder.c Greg Kroah-Hartman
2011-11-30 2462 {
a056af42032e56 drivers/android/binder.c Martijn Coenen
2017-02-03 2463 int ret;
355b0502f6efea drivers/staging/android/binder.c Greg Kroah-Hartman
2011-11-30 2464 struct binder_transaction *t;
44b73962cb25f1 drivers/android/binder.c Sherry Yang
2018-08-13 2465 struct binder_work *w;
355b0502f6efea drivers/staging/android/binder.c Greg Kroah-Hartman
2011-11-30 2466 struct binder_work *tcomplete;
bde4a19fc04f5f drivers/android/binder.c Todd Kjos
2019-02-08 2467 binder_size_t buffer_offset = 0;
bde4a19fc04f5f drivers/android/binder.c Todd Kjos
2019-02-08 2468 binder_size_t off_start_offset, off_end_offset;
212265e5ad726e drivers/android/binder.c Arve Hjønnevåg
2016-02-09 2469 binder_size_t off_min;
bde4a19fc04f5f drivers/android/binder.c Todd Kjos
2019-02-08 2470 binder_size_t sg_buf_offset, sg_buf_end_offset;
d51c5e7a3791e9 drivers/android/binder.c Todd Kjos
2021-11-23 2471 binder_size_t user_offset = 0;
7a4408c6bd3eb1 drivers/android/binder.c Todd Kjos
2017-06-29 2472 struct binder_proc *target_proc = NULL;
355b0502f6efea drivers/staging/android/binder.c Greg Kroah-Hartman
2011-11-30 2473 struct binder_thread *target_thread = NULL;
355b0502f6efea drivers/staging/android/binder.c Greg Kroah-Hartman
2011-11-30 2474 struct binder_node *target_node = NULL;
355b0502f6efea drivers/staging/android/binder.c Greg Kroah-Hartman
2011-11-30 2475 struct binder_transaction *in_reply_to = NULL;
355b0502f6efea drivers/staging/android/binder.c Greg Kroah-Hartman
2011-11-30 2476 struct binder_transaction_log_entry *e;
57ada2fb2250ea drivers/android/bin
Re: [PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
On Thu, Nov 25, 2021 at 12:02:34PM +, Lee Jones wrote:
> Supply additional checks in order to prevent unexpected results.
>
> Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
> Signed-off-by: Lee Jones
> ---
> drivers/staging/android/ion/ion.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/staging/android/ion/ion.c
> b/drivers/staging/android/ion/ion.c
> index 806e9b30b9dc8..30f359faba575 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -509,6 +509,9 @@ static void *ion_handle_kmap_get(struct ion_handle
> *handle)
> void *vaddr;
>
> if (handle->kmap_cnt) {
> + if (handle->kmap_cnt + 1 < handle->kmap_cnt)
What about using the nice helpers in overflow.h for this?
> + return ERR_PTR(-EOVERFLOW);
> +
> handle->kmap_cnt++;
> return buffer->vaddr;
> }
> --
> 2.34.0.rc2.393.gf8c9666880-goog
What stable kernel branch(es) is this for?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] staging: ion: Prevent incorrect reference counting behavour
Supply additional checks in order to prevent unexpected results.
Fixes: b892bf75b2034 ("ion: Switch ion to use dma-buf")
Signed-off-by: Lee Jones
---
drivers/staging/android/ion/ion.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 806e9b30b9dc8..30f359faba575 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -509,6 +509,9 @@ static void *ion_handle_kmap_get(struct ion_handle *handle)
void *vaddr;
if (handle->kmap_cnt) {
+ if (handle->kmap_cnt + 1 < handle->kmap_cnt)
+ return ERR_PTR(-EOVERFLOW);
+
handle->kmap_cnt++;
return buffer->vaddr;
}
--
2.34.0.rc2.393.gf8c9666880-goog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
