FINANCIAL MANAGEMENT AND ADVISORY SERVICES LIMITED 1ม
Dear de...@driverdev.osuosl.org, Please pardon me for this unsolicited communique. I do have the trusteeship of a PRIVATE investor with a stormy political background to outsource individuals with sound Financial Management abilities to manage over US$1.3B devoid of his name. These funds can be invested in tranches of US $100M or a tranche that is suitable for the portfolio manager. If you have Financial Management abilities, credible project in need of funding or existing business requiring expansion, your feedback would be appreciated. Sincerely, Martin Jones Managing Partner FINANCIAL MANAGEMENT AND ADVISORY SERVICES LIMITED ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: prefix header search paths with $(srctree)/
Currently, the Kbuild core manipulates header search paths in a crazy
way [1].
To fix this mess, I want all Makefiles to add explicit $(srctree)/ to
the search paths in the srctree. Some Makefiles are already written in
that way, but not all. The goal of this work is to make the notation
consistent, and finally get rid of the gross hacks.
Having whitespaces after -I does not matter since commit 48f6e3cf5bc6
("kbuild: do not drop -I without parameter").
[1]: https://patchwork.kernel.org/patch/9632347/
Signed-off-by: Masahiro Yamada
---
drivers/staging/erofs/Makefile| 2 +-
drivers/staging/media/davinci_vpfe/Makefile | 2 +-
drivers/staging/most/Makefile | 2 +-
drivers/staging/most/cdev/Makefile| 2 +-
drivers/staging/most/dim2/Makefile| 2 +-
drivers/staging/most/i2c/Makefile | 2 +-
drivers/staging/most/net/Makefile | 2 +-
drivers/staging/most/sound/Makefile | 2 +-
drivers/staging/most/usb/Makefile | 2 +-
drivers/staging/most/video/Makefile | 2 +-
drivers/staging/rtl8192u/Makefile | 2 +-
drivers/staging/unisys/visorhba/Makefile | 3 +--
drivers/staging/unisys/visornic/Makefile | 3 +--
drivers/staging/vc04_services/bcm2835-audio/Makefile | 3 +--
drivers/staging/vc04_services/bcm2835-camera/Makefile | 2 +-
15 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/erofs/Makefile b/drivers/staging/erofs/Makefile
index c91b652..38ab344 100644
--- a/drivers/staging/erofs/Makefile
+++ b/drivers/staging/erofs/Makefile
@@ -6,7 +6,7 @@ ccflags-y += -Wall -DEROFS_VERSION=\"$(EROFS_VERSION)\"
obj-$(CONFIG_EROFS_FS) += erofs.o
# staging requirement: to be self-contained in its own directory
-ccflags-y += -I$(src)/include
+ccflags-y += -I $(srctree)/$(src)/include
erofs-objs := super.o inode.o data.o namei.o dir.o utils.o
erofs-$(CONFIG_EROFS_FS_XATTR) += xattr.o
erofs-$(CONFIG_EROFS_FS_ZIP) += unzip_vle.o unzip_vle_lz4.o
diff --git a/drivers/staging/media/davinci_vpfe/Makefile
b/drivers/staging/media/davinci_vpfe/Makefile
index 9c57042..9268e50 100644
--- a/drivers/staging/media/davinci_vpfe/Makefile
+++ b/drivers/staging/media/davinci_vpfe/Makefile
@@ -6,5 +6,5 @@ davinci-vfpe-objs := \
# Allow building it with COMPILE_TEST on other archs
ifndef CONFIG_ARCH_DAVINCI
-ccflags-y += -Iarch/arm/mach-davinci/include/
+ccflags-y += -I $(srctree)/arch/arm/mach-davinci/include/
endif
diff --git a/drivers/staging/most/Makefile b/drivers/staging/most/Makefile
index f8bcf48..c7662f6 100644
--- a/drivers/staging/most/Makefile
+++ b/drivers/staging/most/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_MOST) += most_core.o
most_core-y := core.o
-ccflags-y += -Idrivers/staging/
+ccflags-y += -I $(srctree)/drivers/staging/
obj-$(CONFIG_MOST_CDEV)+= cdev/
obj-$(CONFIG_MOST_NET) += net/
diff --git a/drivers/staging/most/cdev/Makefile
b/drivers/staging/most/cdev/Makefile
index afb9870..21b0bd7 100644
--- a/drivers/staging/most/cdev/Makefile
+++ b/drivers/staging/most/cdev/Makefile
@@ -1,4 +1,4 @@
obj-$(CONFIG_MOST_CDEV) += most_cdev.o
most_cdev-objs := cdev.o
-ccflags-y += -Idrivers/staging/
+ccflags-y += -I $(srctree)/drivers/staging/
diff --git a/drivers/staging/most/dim2/Makefile
b/drivers/staging/most/dim2/Makefile
index 66676f5..6d15f04 100644
--- a/drivers/staging/most/dim2/Makefile
+++ b/drivers/staging/most/dim2/Makefile
@@ -1,4 +1,4 @@
obj-$(CONFIG_MOST_DIM2) += most_dim2.o
most_dim2-objs := dim2.o hal.o sysfs.o
-ccflags-y += -Idrivers/staging/
+ccflags-y += -I $(srctree)/drivers/staging/
diff --git a/drivers/staging/most/i2c/Makefile
b/drivers/staging/most/i2c/Makefile
index a7d094c..c032fea 100644
--- a/drivers/staging/most/i2c/Makefile
+++ b/drivers/staging/most/i2c/Makefile
@@ -1,4 +1,4 @@
obj-$(CONFIG_MOST_I2C) += most_i2c.o
most_i2c-objs := i2c.o
-ccflags-y += -Idrivers/staging/
+ccflags-y += -I $(srctree)/drivers/staging/
diff --git a/drivers/staging/most/net/Makefile
b/drivers/staging/most/net/Makefile
index 54500aa..820faec 100644
--- a/drivers/staging/most/net/Makefile
+++ b/drivers/staging/most/net/Makefile
@@ -1,4 +1,4 @@
obj-$(CONFIG_MOST_NET) += most_net.o
most_net-objs := net.o
-ccflags-y += -Idrivers/staging/
+ccflags-y += -I $(srctree)/drivers/staging/
diff --git a/drivers/staging/most/sound/Makefile
b/drivers/staging/most/sound/Makefile
index eee8774..5bb55bb 100644
--- a/drivers/staging/most/sound/Makefile
+++ b/drivers/staging/most/sound/Makefile
@@ -1,4 +1,4 @@
obj-$(CONFIG_MOST_SOUND) += most_sound.o
most_sound-objs := sound.o
-ccflags-y += -Idrivers/staging/
+ccflags-y += -I $(srctree)/drivers/staging/
diff --git a/drivers/staging/most/usb/Makefile
b/drivers/staging/most/usb/Makefile
index 18d28cb..910cd08 100644
--- a/drivers/staging/most/usb/Makefile
Re: [PATCH] tty: Fix WARNING in tty_set_termios
On 1/30/19 3:32 AM, Johan Hovold wrote:
On Mon, Jan 28, 2019 at 02:29:22PM -0700, shuah wrote:
On 1/25/19 9:14 PM, Al Viro wrote:
On Fri, Jan 25, 2019 at 04:29:05PM -0700, Shuah Khan wrote:
tty_set_termios() has the following WARMN_ON which can be triggered with a
syscall to invoke TIOCGETD __NR_ioctl.
You meant TIOCSETD here, and in fact its the call which sets the uart
protocol that triggers the warning.
Right. It is a TIOCSETD.
WARN_ON(tty->driver->type == TTY_DRIVER_TYPE_PTY &&
tty->driver->subtype == PTY_TYPE_MASTER);
Reference:
https://syzkaller.appspot.com/bug?id=2410d22f1d8e5984217329dd0884b01d99e3e48d
A simple change would have been to print error message instead of WARN_ON.
However, the callers assume that tty_set_termios() always returns 0 and
don't check return value. The complete solution is fixing all the callers
to check error and bail out to fix the WARN_ON.
This fix changes tty_set_termios() to return error and all the callers
to check error and bail out. The reproducer is used to reproduce the
problem and verify the fix.
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -321,6 +321,8 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool
enable)
status = tty_set_termios(tty, );
BT_DBG("Disabling hardware flow control: %s",
status ? "failed" : "success");
+ if (status)
+ return;
Can that ldisc end up set on pty master? And does it make any sense there?
The initial objective of the patch is to prevent the WARN_ON by making
the change to return error instead of WARN_ON. However, without changes
to places that don't check the return and keep making progress, there
will be secondary problems.
Without this change to return here, instead of WARN_ON, it will fail
with the following NULL pointer dereference at the next thing
hci_uart_set_flow_control() attempts.
status = tty->driver->ops->tiocmget(tty);
kernel: [10140.649783] BUG: unable to handle kernel NULL pointer
That's a separate issue, which is being fixed:
https://lkml.kernel.org/r/20190130095938.GP3691@localhost
Ah good to know.
IOW, I don't believe that this patch makes any sense. If anything,
we need to prevent unconditional tty_set_termios() on the path
that *does* lead to calling it for pty.
I don't think preventing unconditional tty_set_termios() is enough to
prevent secondary problems such as the one above.
For example, the following call chain leads to the WARN_ON that was
reported. Even if void hci_uart_set_baudrate() prevents the very first
tty_set_termios() call, its caller hci_uart_setup() continues with
more tty setup. It goes ahead to call driver setup callback. The
driver callback goes on to do more setup calling tty_set_termios().
WARN_ON call path:
hci_uart_set_baudrate+0x1cc/0x250 drivers/bluetooth/hci_ldisc.c:378
hci_uart_setup+0xa2/0x490 drivers/bluetooth/hci_ldisc.c:401
hci_dev_do_open+0x6b1/0x1920 net/bluetooth/hci_core.c:1423
Once this WARN_ON is changed to return error, the following
happens, when hci_uart_setup() does driver setup callback.
kernel: [10140.649836] mrvl_setup+0x17/0x80 [hci_uart]
kernel: [10140.649840] hci_uart_setup+0x56/0x160 [hci_uart]
kernel: [10140.649850] hci_dev_do_open+0xe6/0x630 [bluetooth]
kernel: [10140.649860] hci_power_on+0x52/0x220 [bluetooth]
I think continuing to catch the invalid condition in tty_set_termios()
and preventing progress by checking return value is a straight forward
change to avoid secondary problems, and it might be difficult to catch
all the cases where it could fail.
I agree with Al that this change doesn't make much sense. The WARN_ON
is there to catch any bugs leading to the termios being changed for a
master side pty. Those should bugs should be fixed, and not worked
around in order to silence a WARN_ON.
The problem started with 7721383f4199 ("Bluetooth: hci_uart: Support
operational speed during setup") which introduced a new way for how
tty_set_termios() could end up being called for a master pty.
Ah. Thanks for the context.
As Al hinted at, setting these ldiscs for a master pty really makes no
sense and perhaps that is what we should prevent unless simply making
sure they do not call tty_set_termios() is sufficient for the time
being.
I will take a look to see if not calling tty_set_termios() is enough.
Finally, note that serdev never operates on a pty, and that this is only
an issue for (the three) line disciplines.
Thanks for the detailed message explaining the evolution. Now it makes
sense.
-- Shuah
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] binderfs: remove separate device_initcall()
On Wed, Jan 30, 2019 at 10:17:39PM +0100, Greg KH wrote:
> On Wed, Jan 30, 2019 at 06:01:02PM +0100, Christian Brauner wrote:
> > On Wed, Jan 30, 2019 at 03:24:12PM +0100, Greg KH wrote:
> > > On Wed, Jan 23, 2019 at 12:41:16PM +0100, Christian Brauner wrote:
> > > > binderfs should not have a separate device_initcall(). When a kernel is
> > > > compiled with CONFIG_ANDROID_BINDERFS register the filesystem alongside
> > > > CONFIG_ANDROID_IPC. This use-case is especially sensible when users
> > > > specify
> > > > CONFIG_ANDROID_IPC=y, CONFIG_ANDROID_BINDERFS=y and
> > > > ANDROID_BINDER_DEVICES="".
> > > > When CONFIG_ANDROID_BINDERFS=n then this always succeeds so there's no
> > > > regression potential for legacy workloads.
> > > >
> > > > Signed-off-by: Christian Brauner
> > > > ---
> > > > drivers/android/binder.c | 4
> > > > drivers/android/binder_internal.h | 9 +
> > > > drivers/android/binderfs.c| 4 +---
> > > > 3 files changed, 14 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > > > index cdfc87629efb..751d76173f81 100644
> > > > --- a/drivers/android/binder.c
> > > > +++ b/drivers/android/binder.c
> > > > @@ -5915,6 +5915,10 @@ static int __init binder_init(void)
> > > > goto err_init_binder_device_failed;
> > > > }
> > > >
> > > > + ret = init_binderfs();
> > > > + if (ret)
> > > > + goto err_init_binder_device_failed;
> > > > +
> > > > return ret;
> > > >
> > > > err_init_binder_device_failed:
> > > > diff --git a/drivers/android/binder_internal.h
> > > > b/drivers/android/binder_internal.h
> > > > index 7fb97f503ef2..045b3e42d98b 100644
> > > > --- a/drivers/android/binder_internal.h
> > > > +++ b/drivers/android/binder_internal.h
> > > > @@ -46,4 +46,13 @@ static inline bool is_binderfs_device(const struct
> > > > inode *inode)
> > > > }
> > > > #endif
> > > >
> > > > +#ifdef CONFIG_ANDROID_BINDERFS
> > > > +extern int __init init_binderfs(void);
> > > > +#else
> > > > +static inline int __init init_binderfs(void)
> > > > +{
> > > > + return 0;
> > > > +}
> > > > +#endif
> > > > +
> > > > #endif /* _LINUX_BINDER_INTERNAL_H */
> > > > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > > > index 7a550104a722..e773f45d19d9 100644
> > > > --- a/drivers/android/binderfs.c
> > > > +++ b/drivers/android/binderfs.c
> > > > @@ -550,7 +550,7 @@ static struct file_system_type binder_fs_type = {
> > > > .fs_flags = FS_USERNS_MOUNT,
> > > > };
> > > >
> > > > -static int __init init_binderfs(void)
> > > > +int __init init_binderfs(void)
> > > > {
> > > > int ret;
> > > >
> > > > @@ -568,5 +568,3 @@ static int __init init_binderfs(void)
> > > >
> > > > return ret;
> > > > }
> > > > -
> > > > -device_initcall(init_binderfs);
> > >
> > > I get a build warning when applying this patch :(
> >
> > Hm, I can't reproduce that build error with this patch applied to what
> > you currently have in char-misc-linus. :(
> > Any chance you can give me the config that produced this warning?
> > I tried with CONFIG_BINDERFS=y and CONFIG_BINDERFS=n.
>
> $ make M=drivers/android
> CC drivers/android/binderfs.o
> CC drivers/android/binder.o
> drivers/android/binder.c: In function ‘binder_init’:
> drivers/android/binder.c:5933:2: warning: ‘device_names’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> kfree(device_names);
> ^~~
>
> $ gcc --version
> gcc (GCC) 8.2.1 20181127
>
> And gcc is right about this warning, that could happen with your change :(
Thanks for the pointer. New version sent out that fixes this issue! :)
Thanks!
Christian
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v1] binderfs: remove separate device_initcall()
binderfs should not have a separate device_initcall(). When a kernel is
compiled with CONFIG_ANDROID_BINDERFS register the filesystem alongside
CONFIG_ANDROID_IPC. This use-case is especially sensible when users specify
CONFIG_ANDROID_IPC=y, CONFIG_ANDROID_BINDERFS=y and
ANDROID_BINDER_DEVICES="".
When CONFIG_ANDROID_BINDERFS=n then this always succeeds so there's no
regression potential for legacy workloads.
Signed-off-by: Christian Brauner
---
/* Changelog */
- ensure that device_name is set to NULL so kfree() doesn't freak out
---
drivers/android/binder.c | 7 ++-
drivers/android/binder_internal.h | 9 +
drivers/android/binderfs.c| 4 +---
3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 57cf259de600..4d2b2ad1ee0e 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5854,9 +5854,10 @@ static int __init init_binder_device(const char *name)
static int __init binder_init(void)
{
int ret;
- char *device_name, *device_names, *device_tmp;
+ char *device_name, *device_tmp;
struct binder_device *device;
struct hlist_node *tmp;
+ char *device_names = NULL;
ret = binder_alloc_shrinker_init();
if (ret)
@@ -5917,6 +5918,10 @@ static int __init binder_init(void)
}
}
+ ret = init_binderfs();
+ if (ret)
+ goto err_init_binder_device_failed;
+
return ret;
err_init_binder_device_failed:
diff --git a/drivers/android/binder_internal.h
b/drivers/android/binder_internal.h
index 7fb97f503ef2..045b3e42d98b 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -46,4 +46,13 @@ static inline bool is_binderfs_device(const struct inode
*inode)
}
#endif
+#ifdef CONFIG_ANDROID_BINDERFS
+extern int __init init_binderfs(void);
+#else
+static inline int __init init_binderfs(void)
+{
+ return 0;
+}
+#endif
+
#endif /* _LINUX_BINDER_INTERNAL_H */
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 7a550104a722..e773f45d19d9 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -550,7 +550,7 @@ static struct file_system_type binder_fs_type = {
.fs_flags = FS_USERNS_MOUNT,
};
-static int __init init_binderfs(void)
+int __init init_binderfs(void)
{
int ret;
@@ -568,5 +568,3 @@ static int __init init_binderfs(void)
return ret;
}
-
-device_initcall(init_binderfs);
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: patch "staging: erofs: keep corrupted fs from crashing kernel in" added to staging-linus
Hi Dan and Greg, On 2019/1/31 4:00, Dan Carpenter wrote: > On Wed, Jan 30, 2019 at 11:42:41PM +0800, Gao Xiang wrote: >> >> On 2019/1/30 23:05, Gao Xiang wrote: >>> Hi Greg, >>> >>> Dan raised some suggestions to me. And I want to get some review ideas from >>> Chao... >>> Current EROFS works good for the normal image, this patch is used for >>> corrupted image only... >>> >>> Could you kindly drop this patch temporarily and I want to get them >>> reviewed... >>> Not soon... Thanks! >> It seems this patch is the top of staging-linus...Could you kindly drop it >> and >> it is better to get "Reviewed-by:" tag for all EROFS bugfix patches from >> corresponding >> people (Chao, Dan, or Al if possible) first... Thank you very much! >> >> sorry to trouble you... >> > Greg has already reverted this but for future reference there wasn't > any need to panic or rush. It was just the kunmap_atomic() which only > affects corrupted filesystem on linux-next so it's not a big deal. The > rest was just my style grumblings and could have been addressed later or > even ignored. > > regards, > dan carpenter Although I test many test images and do Android boot in my internal test with this patch, I am afraid there are some potential issues which haven't found (The old implementation is stable enough since it works good for months and it has been applied to our real products... This patch was just written days ago) It is better to get some "Reviewed-by" tag for such patches at least by EROFS guys, maybe goto linux-next is better since I don't want to break the current EROFS stability in 5.0-rc round... Thanks, Gao Xiang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/7] binder: avoid kernel vm_area for buffer fixups
Refactor the functions to validate and fixup struct
binder_buffer pointer objects to avoid using vm_area
pointers. Instead copy to/from kernel space using
binder_alloc_copy_to_buffer() and
binder_alloc_copy_from_buffer(). The following
functions were refactored:
binder_validate_ptr()
binder_validate_fixup()
binder_fixup_parent()
Signed-off-by: Todd Kjos
---
drivers/android/binder.c | 146 ++-
1 file changed, 97 insertions(+), 49 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 8063b405e4fa..98163bf5f35c 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2092,10 +2092,13 @@ static size_t binder_get_object(struct binder_proc
*proc,
/**
* binder_validate_ptr() - validates binder_buffer_object in a binder_buffer.
+ * @proc: binder_proc owning the buffer
* @b: binder_buffer containing the object
+ * @object:struct binder_object to read into
* @index: index in offset array at which the binder_buffer_object is
* located
- * @start: points to the start of the offset array
+ * @start_offset: points to the start of the offset array
+ * @object_offsetp: offset of @object read from @b
* @num_valid: the number of valid offsets in the offset array
*
* Return: If @index is within the valid range of the offset array
@@ -2106,34 +2109,46 @@ static size_t binder_get_object(struct binder_proc
*proc,
* Note that the offset found in index @index itself is not
* verified; this function assumes that @num_valid elements
* from @start were previously verified to have valid offsets.
+ * If @object_offsetp is non-NULL, then the offset within
+ * @b is written to it.
*/
-static struct binder_buffer_object *binder_validate_ptr(struct binder_buffer
*b,
- binder_size_t index,
- binder_size_t *start,
- binder_size_t num_valid)
+static struct binder_buffer_object *binder_validate_ptr(
+ struct binder_proc *proc,
+ struct binder_buffer *b,
+ struct binder_object *object,
+ binder_size_t index,
+ binder_size_t start_offset,
+ binder_size_t *object_offsetp,
+ binder_size_t num_valid)
{
- struct binder_buffer_object *buffer_obj;
- binder_size_t *offp;
+ size_t object_size;
+ binder_size_t object_offset;
+ unsigned long buffer_offset;
if (index >= num_valid)
return NULL;
- offp = start + index;
- buffer_obj = (struct binder_buffer_object *)(b->data + *offp);
- if (buffer_obj->hdr.type != BINDER_TYPE_PTR)
+ buffer_offset = start_offset + sizeof(binder_size_t) * index;
+ binder_alloc_copy_from_buffer(>alloc, _offset,
+ b, buffer_offset, sizeof(object_offset));
+ object_size = binder_get_object(proc, b, object_offset, object);
+ if (!object_size || object->hdr.type != BINDER_TYPE_PTR)
return NULL;
+ if (object_offsetp)
+ *object_offsetp = object_offset;
- return buffer_obj;
+ return >bbo;
}
/**
* binder_validate_fixup() - validates pointer/fd fixups happen in order.
+ * @proc: binder_proc owning the buffer
* @b: transaction buffer
- * @objects_start start of objects buffer
- * @buffer:binder_buffer_object in which to fix up
- * @offset:start offset in @buffer to fix up
- * @last_obj: last binder_buffer_object that we fixed up in
- * @last_min_offset: minimum fixup offset in @last_obj
+ * @objects_start_offset: offset to start of objects buffer
+ * @buffer_obj_offset: offset to binder_buffer_object in which to fix up
+ * @fixup_offset: start offset in @buffer to fix up
+ * @last_obj_offset: offset to last binder_buffer_object that we fixed
+ * @last_min_offset: minimum fixup offset in object at @last_obj_offset
*
* Return: %true if a fixup in buffer @buffer at offset @offset is
* allowed.
@@ -2164,28 +2179,41 @@ static struct binder_buffer_object
*binder_validate_ptr(struct binder_buffer *b,
* C (parent = A, offset = 16)
* D (parent = B, offset = 0) // B is not A or any of A's parents
*/
-static bool binder_validate_fixup(struct binder_buffer *b,
- binder_size_t *objects_start,
- struct binder_buffer_object *buffer,
+static bool
[PATCH v2 5/7] binder: remove kernel vm_area for buffer space
Remove the kernel's vm_area and the code that maps
buffer pages into it.
Signed-off-by: Todd Kjos
---
drivers/android/binder_alloc.c | 40 ++
1 file changed, 2 insertions(+), 38 deletions(-)
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 2eebff4be83e..d4cbe4b3947a 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -265,16 +265,6 @@ static int binder_update_page_range(struct binder_alloc
*alloc, int allocate,
page->alloc = alloc;
INIT_LIST_HEAD(>lru);
- ret = map_kernel_range_noflush((unsigned long)page_addr,
- PAGE_SIZE, PAGE_KERNEL,
- >page_ptr);
- flush_cache_vmap((unsigned long)page_addr,
- (unsigned long)page_addr + PAGE_SIZE);
- if (ret != 1) {
- pr_err("%d: binder_alloc_buf failed to map page at %pK
in kernel\n",
- alloc->pid, page_addr);
- goto err_map_kernel_failed;
- }
user_page_addr =
(uintptr_t)page_addr + alloc->user_buffer_offset;
ret = vm_insert_page(vma, user_page_addr, page[0].page_ptr);
@@ -314,8 +304,6 @@ static int binder_update_page_range(struct binder_alloc
*alloc, int allocate,
continue;
err_vm_insert_page_failed:
- unmap_kernel_range((unsigned long)page_addr, PAGE_SIZE);
-err_map_kernel_failed:
__free_page(page->page_ptr);
page->page_ptr = NULL;
err_alloc_page_failed:
@@ -695,7 +683,6 @@ int binder_alloc_mmap_handler(struct binder_alloc *alloc,
struct vm_area_struct *vma)
{
int ret;
- struct vm_struct *area;
const char *failure_string;
struct binder_buffer *buffer;
@@ -706,28 +693,10 @@ int binder_alloc_mmap_handler(struct binder_alloc *alloc,
goto err_already_mapped;
}
- area = get_vm_area(vma->vm_end - vma->vm_start, VM_ALLOC);
- if (area == NULL) {
- ret = -ENOMEM;
- failure_string = "get_vm_area";
- goto err_get_vm_area_failed;
- }
- alloc->buffer = area->addr;
- alloc->user_buffer_offset =
- vma->vm_start - (uintptr_t)alloc->buffer;
+ alloc->buffer = (void *)vma->vm_start;
+ alloc->user_buffer_offset = 0;
mutex_unlock(_alloc_mmap_lock);
-#ifdef CONFIG_CPU_CACHE_VIPT
- if (cache_is_vipt_aliasing()) {
- while (CACHE_COLOUR(
- (vma->vm_start ^ (uint32_t)alloc->buffer))) {
- pr_info("%s: %d %lx-%lx maps %pK bad alignment\n",
- __func__, alloc->pid, vma->vm_start,
- vma->vm_end, alloc->buffer);
- vma->vm_start += PAGE_SIZE;
- }
- }
-#endif
alloc->pages = kcalloc((vma->vm_end - vma->vm_start) / PAGE_SIZE,
sizeof(alloc->pages[0]),
GFP_KERNEL);
@@ -760,9 +729,7 @@ int binder_alloc_mmap_handler(struct binder_alloc *alloc,
alloc->pages = NULL;
err_alloc_pages_failed:
mutex_lock(_alloc_mmap_lock);
- vfree(alloc->buffer);
alloc->buffer = NULL;
-err_get_vm_area_failed:
err_already_mapped:
mutex_unlock(_alloc_mmap_lock);
binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
@@ -821,12 +788,10 @@ void binder_alloc_deferred_release(struct binder_alloc
*alloc)
"%s: %d: page %d at %pK %s\n",
__func__, alloc->pid, i, page_addr,
on_lru ? "on lru" : "active");
- unmap_kernel_range((unsigned long)page_addr, PAGE_SIZE);
__free_page(alloc->pages[i].page_ptr);
page_count++;
}
kfree(alloc->pages);
- vfree(alloc->buffer);
}
mutex_unlock(>mutex);
if (alloc->vma_vm_mm)
@@ -988,7 +953,6 @@ enum lru_status binder_alloc_free_page(struct list_head
*item,
trace_binder_unmap_kernel_start(alloc, index);
- unmap_kernel_range(page_addr, PAGE_SIZE);
__free_page(page->page_ptr);
page->page_ptr = NULL;
--
2.20.1.495.gaa96b0ce6b-goog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/7] binder: remove user_buffer_offset
Remove user_buffer_offset since there is no kernel
buffer pointer anymore.
Signed-off-by: Todd Kjos
---
v2: removed casts as suggested by Dan Carpenter
drivers/android/binder.c | 39 ++
drivers/android/binder_alloc.c | 16 ++
drivers/android/binder_alloc.h | 23
3 files changed, 13 insertions(+), 65 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 98163bf5f35c..b3d609b5935a 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2380,7 +2380,6 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
struct binder_fd_array_object *fda;
struct binder_buffer_object *parent;
struct binder_object ptr_object;
- uintptr_t parent_buffer;
u32 *fd_array;
size_t fd_index;
binder_size_t fd_buf_size;
@@ -2405,14 +2404,6 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
debug_id);
continue;
}
- /*
-* Since the parent was already fixed up, convert it
-* back to kernel address space to access it
-*/
- parent_buffer = parent->buffer -
- binder_alloc_get_user_buffer_offset(
- >alloc);
-
fd_buf_size = sizeof(u32) * fda->num_fds;
if (fda->num_fds >= SIZE_MAX / sizeof(u32)) {
pr_err("transaction release %d invalid number
of fds (%lld)\n",
@@ -2426,7 +2417,8 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
debug_id, (u64)fda->num_fds);
continue;
}
- fd_array = (u32 *)(parent_buffer +
(uintptr_t)fda->parent_offset);
+ fd_array = (u32 *)(uintptr_t)
+ (parent->buffer + fda->parent_offset);
for (fd_index = 0; fd_index < fda->num_fds;
fd_index++) {
u32 fd;
@@ -2646,7 +2638,6 @@ static int binder_translate_fd_array(struct
binder_fd_array_object *fda,
struct binder_transaction *in_reply_to)
{
binder_size_t fdi, fd_buf_size;
- uintptr_t parent_buffer;
u32 *fd_array;
struct binder_proc *proc = thread->proc;
struct binder_proc *target_proc = t->to_proc;
@@ -2664,13 +2655,7 @@ static int binder_translate_fd_array(struct
binder_fd_array_object *fda,
proc->pid, thread->pid, (u64)fda->num_fds);
return -EINVAL;
}
- /*
-* Since the parent was already fixed up, convert it
-* back to the kernel address space to access it
-*/
- parent_buffer = parent->buffer -
- binder_alloc_get_user_buffer_offset(_proc->alloc);
- fd_array = (u32 *)(parent_buffer + (uintptr_t)fda->parent_offset);
+ fd_array = (u32 *)(uintptr_t)(parent->buffer + fda->parent_offset);
if (!IS_ALIGNED((unsigned long)fd_array, sizeof(u32))) {
binder_user_error("%d:%d parent offset not aligned
correctly.\n",
proc->pid, thread->pid);
@@ -2703,7 +2688,6 @@ static int binder_fixup_parent(struct binder_transaction
*t,
binder_size_t last_fixup_min_off)
{
struct binder_buffer_object *parent;
- u8 *parent_buffer;
struct binder_buffer *b = t->buffer;
struct binder_proc *proc = thread->proc;
struct binder_proc *target_proc = t->to_proc;
@@ -2739,11 +2723,8 @@ static int binder_fixup_parent(struct binder_transaction
*t,
proc->pid, thread->pid);
return -EINVAL;
}
- parent_buffer = (u8 *)((uintptr_t)parent->buffer -
- binder_alloc_get_user_buffer_offset(
- _proc->alloc));
buffer_offset = bp->parent_offset +
- (uintptr_t)parent_buffer - (uintptr_t)b->data;
+ (uintptr_t)parent->buffer - (uintptr_t)b->data;
binder_alloc_copy_to_buffer(_proc->alloc, b, buffer_offset,
>buffer, sizeof(bp->buffer));
@@ -3159,10 +3140,8 @@ static void binder_transaction(struct binder_proc *proc,
ALIGN(tr->offsets_size, sizeof(void *)) +
ALIGN(extra_buffers_size, sizeof(void *)) -
ALIGN(secctx_sz, sizeof(u64));
-
[PATCH v2 0/7] binder: eliminate use of vmalloc space for binder buffers
Binder buffers have always been mapped into kernel space via map_kernel_range_noflush() to allow the binder driver to modify the buffer before posting to userspace for processing. In recent Android releases, the number of long-running binder processes has increased to the point that for 32-bit systems, there is a risk of running out of vmalloc space. This patch set removes the persistent mapping of the binder buffers into kernel space. Instead, the binder driver creates temporary mappings with kmap() or kmap_atomic() to copy to or from the buffer only when necessary. Todd Kjos (7): binder: create userspace-to-binder-buffer copy function binder: add functions to copy to/from binder buffers binder: add function to copy binder object from buffer binder: avoid kernel vm_area for buffer fixups binder: remove kernel vm_area for buffer space binder: remove user_buffer_offset binder: use userspace pointer as base of buffer space v2: remove casts as suggested by Dan Carpenter drivers/android/Kconfig| 2 +- drivers/android/binder.c | 460 +--- drivers/android/binder_alloc.c | 299 +- drivers/android/binder_alloc.h | 47 +- drivers/android/binder_trace.h | 2 +- 5 files changed, 534 insertions(+), 276 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 7/7] binder: use userspace pointer as base of buffer space
Now that alloc->buffer points to the userspace vm_area
rename buffer->data to buffer->user_data and rename
local pointers that hold user addresses. Also use the
"__user" tag to annotate all user pointers so sparse
can flag cases where user pointer vaues are copied to
kernel pointers. Refactor code to use offsets instead
of user pointers.
Signed-off-by: Todd Kjos
---
v2: removed casts as suggested by Dan Carpenter
drivers/android/binder.c | 118 +++--
drivers/android/binder_alloc.c | 87
drivers/android/binder_alloc.h | 6 +-
drivers/android/binder_trace.h | 2 +-
4 files changed, 116 insertions(+), 97 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index b3d609b5935a..25491eceb750 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2278,33 +2278,30 @@ static void binder_deferred_fd_close(int fd)
static void binder_transaction_buffer_release(struct binder_proc *proc,
struct binder_buffer *buffer,
- binder_size_t *failed_at)
+ binder_size_t failed_at,
+ bool is_failure)
{
- binder_size_t *offp, *off_start, *off_end;
int debug_id = buffer->debug_id;
- binder_size_t off_start_offset;
+ binder_size_t off_start_offset, buffer_offset, off_end_offset;
binder_debug(BINDER_DEBUG_TRANSACTION,
-"%d buffer release %d, size %zd-%zd, failed at %pK\n",
+"%d buffer release %d, size %zd-%zd, failed at %llx\n",
proc->pid, buffer->debug_id,
-buffer->data_size, buffer->offsets_size, failed_at);
+buffer->data_size, buffer->offsets_size,
+(unsigned long long)failed_at);
if (buffer->target_node)
binder_dec_node(buffer->target_node, 1, 0);
off_start_offset = ALIGN(buffer->data_size, sizeof(void *));
- off_start = (binder_size_t *)(buffer->data + off_start_offset);
- if (failed_at)
- off_end = failed_at;
- else
- off_end = (void *)off_start + buffer->offsets_size;
- for (offp = off_start; offp < off_end; offp++) {
+ off_end_offset = is_failure ? failed_at :
+ off_start_offset + buffer->offsets_size;
+ for (buffer_offset = off_start_offset; buffer_offset < off_end_offset;
+buffer_offset += sizeof(binder_size_t)) {
struct binder_object_header *hdr;
size_t object_size;
struct binder_object object;
binder_size_t object_offset;
- binder_size_t buffer_offset = (uintptr_t)offp -
- (uintptr_t)buffer->data;
binder_alloc_copy_from_buffer(>alloc, _offset,
buffer, buffer_offset,
@@ -2380,9 +2377,10 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
struct binder_fd_array_object *fda;
struct binder_buffer_object *parent;
struct binder_object ptr_object;
- u32 *fd_array;
+ binder_size_t fda_offset;
size_t fd_index;
binder_size_t fd_buf_size;
+ binder_size_t num_valid;
if (proc->tsk != current->group_leader) {
/*
@@ -2393,12 +2391,14 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
continue;
}
+ num_valid = (buffer_offset - off_start_offset) /
+ sizeof(binder_size_t);
fda = to_binder_fd_array_object(hdr);
parent = binder_validate_ptr(proc, buffer, _object,
fda->parent,
off_start_offset,
NULL,
-offp - off_start);
+num_valid);
if (!parent) {
pr_err("transaction release %d bad parent
offset\n",
debug_id);
@@ -2417,14 +2417,21 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
debug_id, (u64)fda->num_fds);
continue;
}
- fd_array = (u32 *)(uintptr_t)
- (parent->buffer + fda->parent_offset);
+ /*
[PATCH v2 1/7] binder: create userspace-to-binder-buffer copy function
The binder driver uses a vm_area to map the per-process
binder buffer space. For 32-bit android devices, this is
now taking too much vmalloc space. This patch removes
the use of vm_area when copying the transaction data
from the sender to the buffer space. Instead of using
copy_from_user() for multi-page copies, it now uses
binder_alloc_copy_user_to_buffer() which uses kmap()
and kunmap() to map each page, and uses copy_from_user()
for copying to that page.
Signed-off-by: Todd Kjos
---
v2: remove casts as suggested by Dan Carpenter
drivers/android/binder.c | 29 +++--
drivers/android/binder_alloc.c | 113 +
drivers/android/binder_alloc.h | 8 +++
3 files changed, 143 insertions(+), 7 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 5f6ef5e63b91..ab0b3eec363b 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3078,8 +3078,12 @@ static void binder_transaction(struct binder_proc *proc,
ALIGN(tr->data_size, sizeof(void *)));
offp = off_start;
- if (copy_from_user(t->buffer->data, (const void __user *)(uintptr_t)
- tr->data.ptr.buffer, tr->data_size)) {
+ if (binder_alloc_copy_user_to_buffer(
+ _proc->alloc,
+ t->buffer, 0,
+ (const void __user *)
+ (uintptr_t)tr->data.ptr.buffer,
+ tr->data_size)) {
binder_user_error("%d:%d got transaction with invalid data
ptr\n",
proc->pid, thread->pid);
return_error = BR_FAILED_REPLY;
@@ -3087,8 +3091,13 @@ static void binder_transaction(struct binder_proc *proc,
return_error_line = __LINE__;
goto err_copy_data_failed;
}
- if (copy_from_user(offp, (const void __user *)(uintptr_t)
- tr->data.ptr.offsets, tr->offsets_size)) {
+ if (binder_alloc_copy_user_to_buffer(
+ _proc->alloc,
+ t->buffer,
+ ALIGN(tr->data_size, sizeof(void *)),
+ (const void __user *)
+ (uintptr_t)tr->data.ptr.offsets,
+ tr->offsets_size)) {
binder_user_error("%d:%d got transaction with invalid offsets
ptr\n",
proc->pid, thread->pid);
return_error = BR_FAILED_REPLY;
@@ -3217,6 +3226,8 @@ static void binder_transaction(struct binder_proc *proc,
struct binder_buffer_object *bp =
to_binder_buffer_object(hdr);
size_t buf_left = sg_buf_end - sg_bufp;
+ binder_size_t sg_buf_offset = (uintptr_t)sg_bufp -
+ (uintptr_t)t->buffer->data;
if (bp->length > buf_left) {
binder_user_error("%d:%d got transaction with
too large buffer\n",
@@ -3226,9 +3237,13 @@ static void binder_transaction(struct binder_proc *proc,
return_error_line = __LINE__;
goto err_bad_offset;
}
- if (copy_from_user(sg_bufp,
- (const void __user *)(uintptr_t)
- bp->buffer, bp->length)) {
+ if (binder_alloc_copy_user_to_buffer(
+ _proc->alloc,
+ t->buffer,
+ sg_buf_offset,
+ (const void __user *)
+ (uintptr_t)bp->buffer,
+ bp->length)) {
binder_user_error("%d:%d got transaction with
invalid offsets ptr\n",
proc->pid, thread->pid);
return_error_param = -EFAULT;
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 022cd80e80cc..94c0d85c4e75 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -29,6 +29,8 @@
#include
#include
#include
+#include
+#include
#include "binder_alloc.h"
#include "binder_trace.h"
@@ -1053,3 +1055,114 @@ int binder_alloc_shrinker_init(void)
}
return ret;
}
+
+/**
+ * check_buffer() - verify that buffer/offset is safe to access
+ * @alloc: binder_alloc for this proc
+ * @buffer: binder buffer to be accessed
+ * @offset: offset into @buffer data
+ * @bytes: bytes to access from offset
+ *
+ * Check that the @offset/@bytes
[PATCH v2 2/7] binder: add functions to copy to/from binder buffers
Avoid vm_area when copying to or from binder buffers.
Instead, new copy functions are added that copy from
kernel space to binder buffer space. These use
kmap_atomic() and kunmap_atomic() to create temporary
mappings and then memcpy() is used to copy within
that page.
Also, kmap_atomic() / kunmap_atomic() use the appropriate
cache flushing to support VIVT cache architectures.
Allow binder to build if CPU_CACHE_VIVT is defined.
Several uses of the new functions are added here. More
to follow in subsequent patches.
Signed-off-by: Todd Kjos
---
v2: remove casts as suggested by Dan Carpenter
drivers/android/Kconfig| 2 +-
drivers/android/binder.c | 119 +
drivers/android/binder_alloc.c | 59
drivers/android/binder_alloc.h | 12
4 files changed, 147 insertions(+), 45 deletions(-)
diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
index 4c190f8d1f4c..6fdf2abe4598 100644
--- a/drivers/android/Kconfig
+++ b/drivers/android/Kconfig
@@ -10,7 +10,7 @@ if ANDROID
config ANDROID_BINDER_IPC
bool "Android Binder IPC Driver"
- depends on MMU && !CPU_CACHE_VIVT
+ depends on MMU
default n
---help---
Binder is used in Android for both communication between processes,
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index ab0b3eec363b..74d0c1ff874e 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2244,14 +2244,22 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
off_end = (void *)off_start + buffer->offsets_size;
for (offp = off_start; offp < off_end; offp++) {
struct binder_object_header *hdr;
- size_t object_size = binder_validate_object(buffer, *offp);
-
+ size_t object_size;
+ binder_size_t object_offset;
+ binder_size_t buffer_offset = (uintptr_t)offp -
+ (uintptr_t)buffer->data;
+
+ binder_alloc_copy_from_buffer(>alloc, _offset,
+ buffer, buffer_offset,
+ sizeof(object_offset));
+ object_size = binder_validate_object(buffer, object_offset);
if (object_size == 0) {
pr_err("transaction release %d bad object at offset
%lld, size %zd\n",
- debug_id, (u64)*offp, buffer->data_size);
+ debug_id, (u64)object_offset, buffer->data_size);
continue;
}
- hdr = (struct binder_object_header *)(buffer->data + *offp);
+ hdr = (struct binder_object_header *)
+ (buffer->data + object_offset);
switch (hdr->type) {
case BINDER_TYPE_BINDER:
case BINDER_TYPE_WEAK_BINDER: {
@@ -2359,8 +2367,20 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
continue;
}
fd_array = (u32 *)(parent_buffer +
(uintptr_t)fda->parent_offset);
- for (fd_index = 0; fd_index < fda->num_fds; fd_index++)
- binder_deferred_fd_close(fd_array[fd_index]);
+ for (fd_index = 0; fd_index < fda->num_fds;
+fd_index++) {
+ u32 fd;
+ binder_size_t offset =
+ (uintptr_t)_array[fd_index] -
+ (uintptr_t)buffer->data;
+
+ binder_alloc_copy_from_buffer(>alloc,
+ ,
+ buffer,
+ offset,
+ sizeof(fd));
+ binder_deferred_fd_close(fd);
+ }
} break;
default:
pr_err("transaction release %d bad object type %x\n",
@@ -2496,7 +2516,7 @@ static int binder_translate_handle(struct
flat_binder_object *fp,
return ret;
}
-static int binder_translate_fd(u32 *fdp,
+static int binder_translate_fd(u32 fd, binder_size_t fd_offset,
struct binder_transaction *t,
struct binder_thread *thread,
struct binder_transaction *in_reply_to)
@@ -2507,7 +2527,6 @@ static int binder_translate_fd(u32 *fdp,
struct file *file;
int ret = 0;
bool target_allows_fd;
- int fd = *fdp;
if (in_reply_to)
target_allows_fd = !!(in_reply_to->flags & TF_ACCEPT_FDS);
@@ -2546,7 +2565,7 @@ static int
[PATCH v2 3/7] binder: add function to copy binder object from buffer
When creating or tearing down a transaction, the binder driver
examines objects in the buffer and takes appropriate action.
To do this without needing to dereference pointers into the
buffer, the local copies of the objects are needed. This patch
introduces a function to validate and copy binder objects
from the buffer to a local structure.
Signed-off-by: Todd Kjos
---
v2: remove casts as suggested by Dan Carpenter
drivers/android/binder.c | 75 +++-
1 file changed, 58 insertions(+), 17 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 74d0c1ff874e..8063b405e4fa 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -628,6 +628,26 @@ struct binder_transaction {
spinlock_t lock;
};
+/**
+ * struct binder_object - union of flat binder object types
+ * @hdr: generic object header
+ * @fbo: binder object (nodes and refs)
+ * @fdo: file descriptor object
+ * @bbo: binder buffer pointer
+ * @fdao: file descriptor array
+ *
+ * Used for type-independent object copies
+ */
+struct binder_object {
+ union {
+ struct binder_object_header hdr;
+ struct flat_binder_object fbo;
+ struct binder_fd_object fdo;
+ struct binder_buffer_object bbo;
+ struct binder_fd_array_object fdao;
+ };
+};
+
/**
* binder_proc_lock() - Acquire outer lock for given binder_proc
* @proc: struct binder_proc to acquire
@@ -2017,26 +2037,33 @@ static void binder_cleanup_transaction(struct
binder_transaction *t,
}
/**
- * binder_validate_object() - checks for a valid metadata object in a buffer.
+ * binder_get_object() - gets object and checks for valid metadata
+ * @proc: binder_proc owning the buffer
* @buffer:binder_buffer that we're parsing.
- * @offset:offset in the buffer at which to validate an object.
+ * @offset:offset in the @buffer at which to validate an object.
+ * @object:struct binder_object to read into
*
* Return: If there's a valid metadata object at @offset in @buffer, the
- * size of that object. Otherwise, it returns zero.
+ * size of that object. Otherwise, it returns zero. The object
+ * is read into the struct binder_object pointed to by @object.
*/
-static size_t binder_validate_object(struct binder_buffer *buffer, u64 offset)
+static size_t binder_get_object(struct binder_proc *proc,
+ struct binder_buffer *buffer,
+ unsigned long offset,
+ struct binder_object *object)
{
- /* Check if we can read a header first */
+ size_t read_size;
struct binder_object_header *hdr;
size_t object_size = 0;
- if (buffer->data_size < sizeof(*hdr) ||
- offset > buffer->data_size - sizeof(*hdr) ||
- !IS_ALIGNED(offset, sizeof(u32)))
+ read_size = min_t(size_t, sizeof(*object), buffer->data_size - offset);
+ if (read_size < sizeof(*hdr))
return 0;
+ binder_alloc_copy_from_buffer(>alloc, object, buffer,
+ offset, read_size);
- /* Ok, now see if we can read a complete object. */
- hdr = (struct binder_object_header *)(buffer->data + offset);
+ /* Ok, now see if we read a complete object. */
+ hdr = >hdr;
switch (hdr->type) {
case BINDER_TYPE_BINDER:
case BINDER_TYPE_WEAK_BINDER:
@@ -2245,6 +2272,7 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
for (offp = off_start; offp < off_end; offp++) {
struct binder_object_header *hdr;
size_t object_size;
+ struct binder_object object;
binder_size_t object_offset;
binder_size_t buffer_offset = (uintptr_t)offp -
(uintptr_t)buffer->data;
@@ -2252,14 +2280,14 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
binder_alloc_copy_from_buffer(>alloc, _offset,
buffer, buffer_offset,
sizeof(object_offset));
- object_size = binder_validate_object(buffer, object_offset);
+ object_size = binder_get_object(proc, buffer,
+ object_offset, );
if (object_size == 0) {
pr_err("transaction release %d bad object at offset
%lld, size %zd\n",
debug_id, (u64)object_offset, buffer->data_size);
continue;
}
- hdr = (struct binder_object_header *)
- (buffer->data + object_offset);
+ hdr =
switch (hdr->type) {
case BINDER_TYPE_BINDER:
case
Re: [PATCH 2/2] binderfs: remove separate device_initcall()
On Wed, Jan 30, 2019 at 06:01:02PM +0100, Christian Brauner wrote:
> On Wed, Jan 30, 2019 at 03:24:12PM +0100, Greg KH wrote:
> > On Wed, Jan 23, 2019 at 12:41:16PM +0100, Christian Brauner wrote:
> > > binderfs should not have a separate device_initcall(). When a kernel is
> > > compiled with CONFIG_ANDROID_BINDERFS register the filesystem alongside
> > > CONFIG_ANDROID_IPC. This use-case is especially sensible when users
> > > specify
> > > CONFIG_ANDROID_IPC=y, CONFIG_ANDROID_BINDERFS=y and
> > > ANDROID_BINDER_DEVICES="".
> > > When CONFIG_ANDROID_BINDERFS=n then this always succeeds so there's no
> > > regression potential for legacy workloads.
> > >
> > > Signed-off-by: Christian Brauner
> > > ---
> > > drivers/android/binder.c | 4
> > > drivers/android/binder_internal.h | 9 +
> > > drivers/android/binderfs.c| 4 +---
> > > 3 files changed, 14 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > > index cdfc87629efb..751d76173f81 100644
> > > --- a/drivers/android/binder.c
> > > +++ b/drivers/android/binder.c
> > > @@ -5915,6 +5915,10 @@ static int __init binder_init(void)
> > > goto err_init_binder_device_failed;
> > > }
> > >
> > > + ret = init_binderfs();
> > > + if (ret)
> > > + goto err_init_binder_device_failed;
> > > +
> > > return ret;
> > >
> > > err_init_binder_device_failed:
> > > diff --git a/drivers/android/binder_internal.h
> > > b/drivers/android/binder_internal.h
> > > index 7fb97f503ef2..045b3e42d98b 100644
> > > --- a/drivers/android/binder_internal.h
> > > +++ b/drivers/android/binder_internal.h
> > > @@ -46,4 +46,13 @@ static inline bool is_binderfs_device(const struct
> > > inode *inode)
> > > }
> > > #endif
> > >
> > > +#ifdef CONFIG_ANDROID_BINDERFS
> > > +extern int __init init_binderfs(void);
> > > +#else
> > > +static inline int __init init_binderfs(void)
> > > +{
> > > + return 0;
> > > +}
> > > +#endif
> > > +
> > > #endif /* _LINUX_BINDER_INTERNAL_H */
> > > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > > index 7a550104a722..e773f45d19d9 100644
> > > --- a/drivers/android/binderfs.c
> > > +++ b/drivers/android/binderfs.c
> > > @@ -550,7 +550,7 @@ static struct file_system_type binder_fs_type = {
> > > .fs_flags = FS_USERNS_MOUNT,
> > > };
> > >
> > > -static int __init init_binderfs(void)
> > > +int __init init_binderfs(void)
> > > {
> > > int ret;
> > >
> > > @@ -568,5 +568,3 @@ static int __init init_binderfs(void)
> > >
> > > return ret;
> > > }
> > > -
> > > -device_initcall(init_binderfs);
> >
> > I get a build warning when applying this patch :(
>
> Hm, I can't reproduce that build error with this patch applied to what
> you currently have in char-misc-linus. :(
> Any chance you can give me the config that produced this warning?
> I tried with CONFIG_BINDERFS=y and CONFIG_BINDERFS=n.
$ make M=drivers/android
CC drivers/android/binderfs.o
CC drivers/android/binder.o
drivers/android/binder.c: In function ‘binder_init’:
drivers/android/binder.c:5933:2: warning: ‘device_names’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
kfree(device_names);
^~~
$ gcc --version
gcc (GCC) 8.2.1 20181127
And gcc is right about this warning, that could happen with your change :(
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier
On Wed, Jan 30, 2019 at 08:01:13PM +, Carlos Henrique Lima Melara wrote:
> On 30/01/2019 13:29, gre...@linuxfoundation.org wrote:
> > On Wed, Jan 30, 2019 at 02:48:27PM +, Carlos Henrique Lima Melara wrote:
> >>This patch fix the checkpatch.p1 warning:
> >>
> >>WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> >>+/*
> >
> > Why did you not fix up the indentation that was already mentioned?
>
> The Warning was about the missing SPDX License Identifier, so I added the
> identifier according to the "license-rules.rst" for C source that is:
I mean the indentation of the text in your changelog, nothing to do with
the actual change in your patch below, sorry for the confusion.
> > The SPDX license identifier is added in form of a comment. The comment
> > style depends on the file type::
> >
> > C source: // SPDX-License-Identifier:
> > C header: /* SPDX-License-Identifier: */
> > ASM: /* SPDX-License-Identifier: */
> > scripts: # SPDX-License-Identifier:
> > .rst: .. SPDX-License-Identifier:
> > .dts{i}: // SPDX-License-Identifier:
>
> I think that was the correction needed, right?
Yes.
> Still have a question about the license comment in the "ethtooll.c".
>
> > /* This program is free software; you can redistribute it and/or modify
> > * it under the terms of the GNU General Public License as published by
> > * the Free Software Foundation; version 2 of the License
> > *
> > * This program is distributed in the hope that it will be useful,
> > * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > * GNU General Public License for more details.
> > *
> > * Copyright (C) 2009-2016 John Crispin
> > * Copyright (C) 2009-2016 Felix Fietkau
> > * Copyright (C) 2013-2016 Michael Lee
> > */
>
> It puts 3 spaces between the * and the text, should it be corrected to
> only one space?
If you really want to, but it's not needed.
All of the "boilerplate" license text can also be removed after the SPDX
line is added, if you want to do that. But leave the copyright lines,
that needs to stay.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier
This patch fixes the checkpatch.p1 warning: WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 +/* It includes the SPDX expression for GPL-2.0 according to the text license in the top of the C file. Signed-off-by: Carlos Henrique Lima Melara --- Changes since v2: - Corrects patch changelog - Removes changes in the copyrigth comment Changes since v1: - Corrects the alingment of patch description - Cc to public mailing lists --- drivers/staging/mt7621-eth/ethtool.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/mt7621-eth/ethtool.c b/drivers/staging/mt7621-eth/ethtool.c index 40a7d47be913..8c4228e2c987 100644 --- a/drivers/staging/mt7621-eth/ethtool.c +++ b/drivers/staging/mt7621-eth/ethtool.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; version 2 of the License -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] nfit: Document sysfs interface dirty_shutdown
The new sysfs node was added in Sep 2018 in:
commit 0ead11181fe0 ("acpi, nfit: Collect shutdown status")
Now let's document it.
Signed-off-by: Dexuan Cui
---
Documentation/ABI/testing/sysfs-bus-nfit | 8
1 file changed, 8 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-nfit
b/Documentation/ABI/testing/sysfs-bus-nfit
index a1cb44dcb908..bd6cde8c0ea9 100644
--- a/Documentation/ABI/testing/sysfs-bus-nfit
+++ b/Documentation/ABI/testing/sysfs-bus-nfit
@@ -153,6 +153,14 @@ Description:
subsystem controller vendor.
+What: /sys/bus/nd/devices/nmemX/nfit/dirty_shutdown
+Date: Sep, 2018
+KernelVersion: v4.20
+Contact: linux-nvd...@lists.01.org
+Description:
+ (RO) Unsafe Shutdown Count for the NVDIMM device.
+
+
What: /sys/bus/nd/devices/ndbusX/nfit/revision
Date: Jun, 2015
KernelVersion: v4.2
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: comedi_fops.c: Remove redundant blank line
Removed redunant blank line. Issue found by checkpatch. Signed-off-by: Sandesh Kenjana Ashok --- drivers/staging/comedi/comedi_fops.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c index 38980fad8be4..0caae4a5c471 100644 --- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -1608,7 +1608,6 @@ static int do_insn_ioctl(struct comedi_device *dev, if (copy_from_user(, arg, sizeof(insn))) return -EFAULT; - n_data = max(n_data, insn.n); /* This is where the behavior of insn and insnlist deviate. */ -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier
On 30/01/2019 13:29, gre...@linuxfoundation.org wrote:
> On Wed, Jan 30, 2019 at 02:48:27PM +, Carlos Henrique Lima Melara wrote:
>> This patch fix the checkpatch.p1 warning:
>>
>> WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
>> +/*
>
> Why did you not fix up the indentation that was already mentioned?
The Warning was about the missing SPDX License Identifier, so I added the
identifier according to the "license-rules.rst" for C source that is:
> The SPDX license identifier is added in form of a comment. The comment
> style depends on the file type::
>
> C source:// SPDX-License-Identifier:
> C header:/* SPDX-License-Identifier: */
> ASM: /* SPDX-License-Identifier: */
> scripts: # SPDX-License-Identifier:
> .rst:.. SPDX-License-Identifier:
> .dts{i}: // SPDX-License-Identifier:
I think that was the correction needed, right?
Still have a question about the license comment in the "ethtooll.c".
> /* This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> * the Free Software Foundation; version 2 of the License
> *
> * This program is distributed in the hope that it will be useful,
> * but WITHOUT ANY WARRANTY; without even the implied warranty of
> * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> * GNU General Public License for more details.
> *
> * Copyright (C) 2009-2016 John Crispin
> * Copyright (C) 2009-2016 Felix Fietkau
> * Copyright (C) 2013-2016 Michael Lee
> */
It puts 3 spaces between the * and the text, should it be corrected to
only one space?
thanks, Carlos Melara.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] nfit: Collect shutdown status for NVDIMM_FAMILY_HYPERV
> From: Linux-nvdimm On Behalf Of > Dexuan Cui > Sent: Wednesday, January 30, 2019 12:03 PM > To: Greg KH > Cc: Josh Poulson ; linux-nvd...@lists.01.org; > Haiyang Zhang ; > driverdev-devel@linuxdriverproject.org; Rafael J. Wysocki > ; linux-ker...@vger.kernel.org; Michael Kelley > ; Sasha Levin ; > linux-a...@vger.kernel.org; Ross Zwisler ; Stephen > Hemminger ; Len Brown > Subject: RE: [PATCH] nfit: Collect shutdown status for > NVDIMM_FAMILY_HYPERV > > > From: Greg KH > > Sent: Wednesday, January 30, 2019 11:38 AM > > > > On Wed, Jan 30, 2019 at 06:48:40PM +, Dexuan Cui wrote: > > > > > > Let's expose the info to the userspace (e.g. ntctl) via sysfs. > > > > If you add a new sysfs file, you need to add a new Documentation/ABI/ > > update as well :( > > It's an existing sysfs node, which was originally added by Dan in Sep 2018: > /sys/bus/nd/devices/nmem0/nfit/dirty_shutdown. > > Before this patch, the node doesn't appear when Linux runs on Hyper-V, > and with this patch, the node can appear now. > > However, indeed, the node and the other related nodes have not been > documented in Documentation/ABI/testing/sysfs-bus-nfit yet. > I suppose Dan would add that? Actually the other nodes have been documented, and only the "dirty_shutdown" is missed. It should be straightfoward. Let me make a patch for this. Thanks for the reminder, Greg! Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] nfit: Collect shutdown status for NVDIMM_FAMILY_HYPERV
> From: Greg KH > Sent: Wednesday, January 30, 2019 11:38 AM > > On Wed, Jan 30, 2019 at 06:48:40PM +, Dexuan Cui wrote: > > > > Let's expose the info to the userspace (e.g. ntctl) via sysfs. > > If you add a new sysfs file, you need to add a new Documentation/ABI/ > update as well :( It's an existing sysfs node, which was originally added by Dan in Sep 2018: /sys/bus/nd/devices/nmem0/nfit/dirty_shutdown. Before this patch, the node doesn't appear when Linux runs on Hyper-V, and with this patch, the node can appear now. However, indeed, the node and the other related nodes have not been documented in Documentation/ABI/testing/sysfs-bus-nfit yet. I suppose Dan would add that? Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: patch "staging: erofs: keep corrupted fs from crashing kernel in" added to staging-linus
On Wed, Jan 30, 2019 at 11:42:41PM +0800, Gao Xiang wrote: > > > On 2019/1/30 23:05, Gao Xiang wrote: > > > > Hi Greg, > > > > Dan raised some suggestions to me. And I want to get some review ideas from > > Chao... > > Current EROFS works good for the normal image, this patch is used for > > corrupted image only... > > > > Could you kindly drop this patch temporarily and I want to get them > > reviewed... > > Not soon... Thanks! > > It seems this patch is the top of staging-linus...Could you kindly drop it and > it is better to get "Reviewed-by:" tag for all EROFS bugfix patches from > corresponding > people (Chao, Dan, or Al if possible) first... Thank you very much! > > sorry to trouble you... > Greg has already reverted this but for future reference there wasn't any need to panic or rush. It was just the kunmap_atomic() which only affects corrupted filesystem on linux-next so it's not a big deal. The rest was just my style grumblings and could have been addressed later or even ignored. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] nfit: Collect shutdown status for NVDIMM_FAMILY_HYPERV
On Wed, Jan 30, 2019 at 06:48:40PM +, Dexuan Cui wrote:
>
> See http://www.uefi.org/RFIC_LIST ("Virtual NVDIMM 0x1901"):
> "Get Unsafe Shutdown Count (Function Index 2)".
>
> Let's expose the info to the userspace (e.g. ntctl) via sysfs.
If you add a new sysfs file, you need to add a new Documentation/ABI/
update as well :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:rtlwifi:base.c Fixed line more than 80 characters
On Wed, Jan 30, 2019 at 07:42:03PM +0100, SandeshKa07 wrote:
> From: Sandesh Kenjana Ashok
>
> Lines over 80 characters are adjusted according to standards.
>
> Signed-off-by: Sandesh Kenjana Ashok
> ---
> drivers/staging/rtlwifi/base.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtlwifi/base.c b/drivers/staging/rtlwifi/base.c
> index 35df2cf60619..9781d1b0549e 100644
> --- a/drivers/staging/rtlwifi/base.c
> +++ b/drivers/staging/rtlwifi/base.c
> @@ -2130,7 +2130,8 @@ void rtl_watchdog_wq_callback(void *data)
> if (rtlpriv->link_info.roam_times >= 5) {
> pr_err("AP off, try to reconnect now\n");
> rtlpriv->link_info.roam_times = 0;
> -
> ieee80211_connection_loss(rtlpriv->mac80211.vif);
> + ieee80211_connection_loss
> + (rtlpriv->mac80211.vif);
Ick, that's harder to read now.
checkpatch is a _guide_ do not think it always knows what is best...
sorry,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] nfit: Collect shutdown status for NVDIMM_FAMILY_HYPERV
On Wed, Jan 30, 2019 at 10:49 AM Dexuan Cui wrote:
>
>
> See http://www.uefi.org/RFIC_LIST ("Virtual NVDIMM 0x1901"):
> "Get Unsafe Shutdown Count (Function Index 2)".
>
> Let's expose the info to the userspace (e.g. ntctl) via sysfs.
>
> Signed-off-by: Dexuan Cui
> ---
> drivers/acpi/nfit/core.c | 51 ++
> drivers/acpi/nfit/hyperv.h | 26 +++
> 2 files changed, 77 insertions(+)
> create mode 100644 drivers/acpi/nfit/hyperv.h
Looks ok to me, but if we grow any more of these "shutdown count"
retrievals I'll want to take a look at being able to call
acpi_nfit_ctl() earlier in the init process so that this data
marshaling can be unified.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] nfit: Collect shutdown status for NVDIMM_FAMILY_HYPERV
See http://www.uefi.org/RFIC_LIST ("Virtual NVDIMM 0x1901"):
"Get Unsafe Shutdown Count (Function Index 2)".
Let's expose the info to the userspace (e.g. ntctl) via sysfs.
Signed-off-by: Dexuan Cui
---
drivers/acpi/nfit/core.c | 51 ++
drivers/acpi/nfit/hyperv.h | 26 +++
2 files changed, 77 insertions(+)
create mode 100644 drivers/acpi/nfit/hyperv.h
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index d5a164b6833a..d504da34ce34 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -25,6 +25,7 @@
#include
#include
#include "intel.h"
+#include "hyperv.h"
#include "nfit.h"
/*
@@ -1802,6 +1803,54 @@ __weak void nfit_intel_shutdown_status(struct nfit_mem
*nfit_mem)
}
}
+__weak void nfit_hyperv_shutdown_status(struct nfit_mem *nfit_mem)
+{
+ struct device *dev = _mem->adev->dev;
+ struct nd_hyperv_shutdown_status status;
+ union acpi_object in_buf = {
+ .buffer.type = ACPI_TYPE_BUFFER,
+ .buffer.length = 0,
+ };
+ union acpi_object in_obj = {
+ .package.type = ACPI_TYPE_PACKAGE,
+ .package.count = 1,
+ .package.elements = _buf,
+ };
+ const u8 func = ND_HYPERV_GET_UNSAFE_SHUTDOWN_COUNT;
+ const guid_t *guid = to_nfit_uuid(nfit_mem->family);
+ u8 revid = nfit_dsm_revid(nfit_mem->family, func);
+ struct acpi_device *adev = nfit_mem->adev;
+ acpi_handle handle = adev->handle;
+ union acpi_object *out_obj;
+
+ if ((nfit_mem->dsm_mask & BIT(func)) == 0)
+ return;
+
+ out_obj = acpi_evaluate_dsm(handle, guid, revid, func, _obj);
+ if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER
+|| out_obj->buffer.length < sizeof(status)) {
+ dev_dbg(dev->parent,
+ "%s: failed to get Unsafe Shutdown Count\n",
+ dev_name(dev));
+ ACPI_FREE(out_obj);
+ return;
+ }
+
+ memcpy(, out_obj->buffer.pointer, sizeof(status));
+ ACPI_FREE(out_obj);
+
+ if (status.general_err != 0) {
+ dev_dbg(dev->parent,
+ "%s: failed to get Unsafe Shutdown Count: err=0x%x\n",
+ dev_name(dev), status.status);
+ return;
+ }
+
+ set_bit(NFIT_MEM_DIRTY_COUNT, _mem->flags);
+ nfit_mem->dirty_shutdown = status.shutdown_count;
+}
+
+
static void populate_shutdown_status(struct nfit_mem *nfit_mem)
{
/*
@@ -1811,6 +1860,8 @@ static void populate_shutdown_status(struct nfit_mem
*nfit_mem)
*/
if (nfit_mem->family == NVDIMM_FAMILY_INTEL)
nfit_intel_shutdown_status(nfit_mem);
+ else if (nfit_mem->family == NVDIMM_FAMILY_HYPERV)
+ nfit_hyperv_shutdown_status(nfit_mem);
}
static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
diff --git a/drivers/acpi/nfit/hyperv.h b/drivers/acpi/nfit/hyperv.h
new file mode 100644
index ..c4a25b8624cc
--- /dev/null
+++ b/drivers/acpi/nfit/hyperv.h
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright(c) 2019 Microsoft Corporation. All rights reserved.
+ * Hyper-V specific definitions for _DSM of Hyper-V Virtual NVDIMM
+ *
+ * See http://www.uefi.org/RFIC_LIST (Virtual NVDIMM 0x1901)
+ */
+#ifndef _NFIT_HYPERV_H_
+#define _NFIT_HYPERV_H_
+
+#define ND_HYPERV_GET_UNSAFE_SHUTDOWN_COUNT 2
+
+struct nd_hyperv_shutdown_status {
+ union {
+ u32 status;
+ struct {
+ u16 general_err;
+ u8 func_err;
+ u8 vendor_err;
+ };
+ };
+ u32 shutdown_count;
+} __packed;
+
+
+#endif
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier
> See the patch about all of this from Thomas on lkml yesterday for > why this is the case. Hi Greg Thanks for the info. I had not seen this, and i guess other have not as well. So here is a link to the patch. https://lkml.org/lkml/2019/1/28/1975 Andrew ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging:rtlwifi:base.c Fixed line more than 80 characters
From: Sandesh Kenjana Ashok
Lines over 80 characters are adjusted according to standards.
Signed-off-by: Sandesh Kenjana Ashok
---
drivers/staging/rtlwifi/base.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/rtlwifi/base.c b/drivers/staging/rtlwifi/base.c
index 35df2cf60619..9781d1b0549e 100644
--- a/drivers/staging/rtlwifi/base.c
+++ b/drivers/staging/rtlwifi/base.c
@@ -2130,7 +2130,8 @@ void rtl_watchdog_wq_callback(void *data)
if (rtlpriv->link_info.roam_times >= 5) {
pr_err("AP off, try to reconnect now\n");
rtlpriv->link_info.roam_times = 0;
-
ieee80211_connection_loss(rtlpriv->mac80211.vif);
+ ieee80211_connection_loss
+ (rtlpriv->mac80211.vif);
}
} else {
rtlpriv->link_info.roam_times = 0;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: patch "staging: erofs: keep corrupted fs from crashing kernel in" added to staging-linus
On Wed, Jan 30, 2019 at 11:42:41PM +0800, Gao Xiang wrote: > > > On 2019/1/30 23:05, Gao Xiang wrote: > > > > Hi Greg, > > > > Dan raised some suggestions to me. And I want to get some review ideas from > > Chao... > > Current EROFS works good for the normal image, this patch is used for > > corrupted image only... > > > > Could you kindly drop this patch temporarily and I want to get them > > reviewed... > > Not soon... Thanks! > > It seems this patch is the top of staging-linus...Could you kindly drop it and > it is better to get "Reviewed-by:" tag for all EROFS bugfix patches from > corresponding > people (Chao, Dan, or Al if possible) first... Thank you very much! > > sorry to trouble you... No problem, now reverted. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier
On Wed, Jan 30, 2019 at 05:38:17PM +0100, Andrew Lunn wrote:
> On Wed, Jan 30, 2019 at 02:48:27PM +, Carlos Henrique Lima Melara wrote:
> > This patch fix the checkpatch.p1 warning:
> >
> > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> > +/*
> >
> > It includes the SPDX expression for GPL-2.0 and corrects the comment
> > format to suit the kernel's coding style.
> >
> > Signed-off-by: Carlos (Charles) Henrique Lima Melara
> >
> > ---
> > drivers/staging/mt7621-eth/ethtool.c | 22 --
> > 1 file changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/mt7621-eth/ethtool.c
> > b/drivers/staging/mt7621-eth/ethtool.c
> > index 40a7d47be913..49d417de64c8 100644
> > --- a/drivers/staging/mt7621-eth/ethtool.c
> > +++ b/drivers/staging/mt7621-eth/ethtool.c
> > @@ -1,15 +1,17 @@
> > -/* This program is free software; you can redistribute it and/or modify
> > - * it under the terms of the GNU General Public License as published by
> > - * the Free Software Foundation; version 2 of the License
> > +// SPDX-License-Identifier: GPL-2.0
>
> Hi Carlos
>
> drivers/staging/mt7621-eth$ grep LICENSE *
> gsw_mt7621.c:MODULE_LICENSE("GPL");
> mtk_eth_soc.c:MODULE_LICENSE("GPL");
>
> And include/linux/module.h
> says:
>
> /*
> * The following license idents are currently accepted as indicating free
> * software modules
> *
> * "GPL" [GNU Public License v2 or later]
> * "GPL v2"[GNU Public License v2]
>
> So the SPDX string probably does not match the MODULE_LICENSE.
No, ignore the MODULE_LICENSE marking for this, go by what the text at
the top of the file says. See the patch about all of this from Thomas on
lkml yesterday for why this is the case.
So the patch is fine. If the changelog stuff ever gets fixed...
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] media: staging/intel-ipu3: Implement lock for stream on/off operations
Hi Sakari,
> -Original Message-
> From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com]
> Sent: Wednesday, January 30, 2019 12:59 AM
> To: Mani, Rajmohan
> Cc: Mauro Carvalho Chehab ; Greg Kroah-Hartman
> ; linux-me...@vger.kernel.org;
> de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org; Laurent Pinchart
> ; Jacopo Mondi ;
> Qiu, Tian Shu ; Cao, Bingbu
> ; z...@paasikivi.fi.intel.com; Zhi, Yong
> ; hverk...@xs4all.nl; tf...@chromium.org
> Subject: Re: [PATCH] media: staging/intel-ipu3: Implement lock for stream
> on/off operations
>
> Hi Rajmohan,
>
> On Tue, Jan 29, 2019 at 02:27:36PM -0800, Rajmohan Mani wrote:
> > Currently concurrent stream off operations on ImgU nodes are not
> > synchronized, leading to use-after-free bugs (as reported by KASAN).
> >
> > [ 250.090724] BUG: KASAN: use-after-free in
> > ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu] [ 250.090726] Read of size 8
> > at addr 888127b29bc0 by task yavta/18836 [ 250.090731] Hardware
> > name: HP Soraka/Soraka, BIOS Google_Soraka.10431.17.0 03/22/2018 [
> 250.090732] Call Trace:
> > [ 250.090735] dump_stack+0x6a/0xb1
> > [ 250.090739] print_address_description+0x8e/0x279
> > [ 250.090743] ? ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu] [
> > 250.090746] kasan_report+0x260/0x28a [ 250.090750]
> > ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu] [ 250.090754]
> > ipu3_css_pool_cleanup+0x24/0x37 [ipu3_imgu] [ 250.090759]
> > ipu3_css_pipeline_cleanup+0x61/0xb9 [ipu3_imgu] [ 250.090763]
> > ipu3_css_stop_streaming+0x1f2/0x321 [ipu3_imgu] [ 250.090768]
> > imgu_s_stream+0x94/0x443 [ipu3_imgu] [ 250.090772] ?
> > ipu3_vb2_buf_queue+0x280/0x280 [ipu3_imgu] [ 250.090775] ?
> > vb2_dma_sg_unmap_dmabuf+0x16/0x6f [videobuf2_dma_sg] [ 250.090778]
> ?
> > vb2_buffer_in_use+0x36/0x58 [videobuf2_common] [ 250.090782]
> > ipu3_vb2_stop_streaming+0xf9/0x135 [ipu3_imgu]
> >
> > Implemented a lock to synchronize imgu stream on / off operations and
> > the modification of streaming flag (in struct imgu_device), to prevent
> > these issues.
> >
> > Reported-by: Laurent Pinchart
> > Suggested-by: Laurent Pinchart
> >
> > Signed-off-by: Rajmohan Mani
> > ---
> > drivers/staging/media/ipu3/ipu3-v4l2.c | 6 ++
> > drivers/staging/media/ipu3/ipu3.c | 3 +++
> > drivers/staging/media/ipu3/ipu3.h | 4
> > 3 files changed, 13 insertions(+)
> >
> > diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c
> > b/drivers/staging/media/ipu3/ipu3-v4l2.c
> > index c7936032beb9..cf7e917cd0c8 100644
> > --- a/drivers/staging/media/ipu3/ipu3-v4l2.c
> > +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
> > @@ -507,12 +507,15 @@ static int ipu3_vb2_start_streaming(struct
> vb2_queue *vq, unsigned int count)
> > goto fail_stop_pipeline;
> > }
> >
> > + mutex_lock(>streaming_lock);
> > +
>
> You appear to be using imgu_device.lock (while searching buffers to queue to
> the device) as well as imgu_video_device.lock (qbuf, dqbuf) to serialise
> access
> to imgu_video_device.buffers list.
Ack
> The two locks may be acquired at the same
> time but each by different processes. That needs to be addressed, but
> probably not in this patch.
>
The node specific locks will be used by different processes and all of these
processes
will be competing commonly (and successfully) for the imgu_device lock.
I will look into this more.
> I wonder if it'd be more simple to use imgu->lock here instead of adding a new
> one.
>
Extending imgu->lock here, does not work in this case, as imgu_queue_buffers()
will be stuck acquiring imgu->lock, which was already acquired by
imgu_s_stream()
through ipu3_vb2_start_streaming().
> > /* Start streaming of the whole pipeline now */
> > dev_dbg(dev, "IMGU streaming is ready to start");
> > r = imgu_s_stream(imgu, true);
> > if (!r)
> > imgu->streaming = true;
> >
> > + mutex_unlock(>streaming_lock);
> > return 0;
> >
> > fail_stop_pipeline:
> > @@ -543,6 +546,8 @@ static void ipu3_vb2_stop_streaming(struct
> vb2_queue *vq)
> > dev_err(>pci_dev->dev,
> > "failed to stop subdev streaming\n");
> >
> > + mutex_lock(>streaming_lock);
> > +
> > /* Was this the first node with streaming disabled? */
> > if (imgu->streaming && ipu3_all_nodes_streaming(imgu, node)) {
> > /* Yes, really stop streaming now */ @@ -552,6 +557,7 @@
> static
> > void ipu3_vb2_stop_streaming(struct vb2_queue *vq)
> > imgu->streaming = false;
> > }
> >
> > + mutex_unlock(>streaming_lock);
> > ipu3_return_all_buffers(imgu, node, VB2_BUF_STATE_ERROR);
>
> > media_pipeline_stop(>vdev.entity);
> > }
> > diff --git a/drivers/staging/media/ipu3/ipu3.c
> > b/drivers/staging/media/ipu3/ipu3.c
> > index d521b3afb8b1..2daee51cd845 100644
> > --- a/drivers/staging/media/ipu3/ipu3.c
> > +++ b/drivers/staging/media/ipu3/ipu3.c
> > @@ -635,6 +635,7 @@ static int imgu_pci_probe(struct pci_dev *pci_dev,
> >
Re: [PATCH 2/2] binderfs: remove separate device_initcall()
On Wed, Jan 30, 2019 at 03:24:12PM +0100, Greg KH wrote:
> On Wed, Jan 23, 2019 at 12:41:16PM +0100, Christian Brauner wrote:
> > binderfs should not have a separate device_initcall(). When a kernel is
> > compiled with CONFIG_ANDROID_BINDERFS register the filesystem alongside
> > CONFIG_ANDROID_IPC. This use-case is especially sensible when users specify
> > CONFIG_ANDROID_IPC=y, CONFIG_ANDROID_BINDERFS=y and
> > ANDROID_BINDER_DEVICES="".
> > When CONFIG_ANDROID_BINDERFS=n then this always succeeds so there's no
> > regression potential for legacy workloads.
> >
> > Signed-off-by: Christian Brauner
> > ---
> > drivers/android/binder.c | 4
> > drivers/android/binder_internal.h | 9 +
> > drivers/android/binderfs.c| 4 +---
> > 3 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > index cdfc87629efb..751d76173f81 100644
> > --- a/drivers/android/binder.c
> > +++ b/drivers/android/binder.c
> > @@ -5915,6 +5915,10 @@ static int __init binder_init(void)
> > goto err_init_binder_device_failed;
> > }
> >
> > + ret = init_binderfs();
> > + if (ret)
> > + goto err_init_binder_device_failed;
> > +
> > return ret;
> >
> > err_init_binder_device_failed:
> > diff --git a/drivers/android/binder_internal.h
> > b/drivers/android/binder_internal.h
> > index 7fb97f503ef2..045b3e42d98b 100644
> > --- a/drivers/android/binder_internal.h
> > +++ b/drivers/android/binder_internal.h
> > @@ -46,4 +46,13 @@ static inline bool is_binderfs_device(const struct inode
> > *inode)
> > }
> > #endif
> >
> > +#ifdef CONFIG_ANDROID_BINDERFS
> > +extern int __init init_binderfs(void);
> > +#else
> > +static inline int __init init_binderfs(void)
> > +{
> > + return 0;
> > +}
> > +#endif
> > +
> > #endif /* _LINUX_BINDER_INTERNAL_H */
> > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > index 7a550104a722..e773f45d19d9 100644
> > --- a/drivers/android/binderfs.c
> > +++ b/drivers/android/binderfs.c
> > @@ -550,7 +550,7 @@ static struct file_system_type binder_fs_type = {
> > .fs_flags = FS_USERNS_MOUNT,
> > };
> >
> > -static int __init init_binderfs(void)
> > +int __init init_binderfs(void)
> > {
> > int ret;
> >
> > @@ -568,5 +568,3 @@ static int __init init_binderfs(void)
> >
> > return ret;
> > }
> > -
> > -device_initcall(init_binderfs);
>
> I get a build warning when applying this patch :(
Hm, I can't reproduce that build error with this patch applied to what
you currently have in char-misc-linus. :(
Any chance you can give me the config that produced this warning?
I tried with CONFIG_BINDERFS=y and CONFIG_BINDERFS=n.
Thanks!
Christian
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier
On Wed, Jan 30, 2019 at 02:48:27PM +, Carlos Henrique Lima Melara wrote:
> This patch fix the checkpatch.p1 warning:
>
> WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> +/*
>
> It includes the SPDX expression for GPL-2.0 and corrects the comment
> format to suit the kernel's coding style.
>
> Signed-off-by: Carlos (Charles) Henrique Lima Melara
>
> ---
> drivers/staging/mt7621-eth/ethtool.c | 22 --
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/mt7621-eth/ethtool.c
> b/drivers/staging/mt7621-eth/ethtool.c
> index 40a7d47be913..49d417de64c8 100644
> --- a/drivers/staging/mt7621-eth/ethtool.c
> +++ b/drivers/staging/mt7621-eth/ethtool.c
> @@ -1,15 +1,17 @@
> -/* This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; version 2 of the License
> +// SPDX-License-Identifier: GPL-2.0
Hi Carlos
drivers/staging/mt7621-eth$ grep LICENSE *
gsw_mt7621.c:MODULE_LICENSE("GPL");
mtk_eth_soc.c:MODULE_LICENSE("GPL");
And include/linux/module.h
says:
/*
* The following license idents are currently accepted as indicating free
* software modules
*
* "GPL" [GNU Public License v2 or later]
* "GPL v2"[GNU Public License v2]
So the SPDX string probably does not match the MODULE_LICENSE.
Andrew
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] drivers:staging: Fixed line more than 80 characters.
Lines over 80 characters are adjusted according to standards
Signed-off-by: Sandesh Kenjana Ashok
---
drivers/staging/wlan-ng/cfg80211.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/wlan-ng/cfg80211.c
b/drivers/staging/wlan-ng/cfg80211.c
index e5d7def1f366..7687f619ee34 100644
--- a/drivers/staging/wlan-ng/cfg80211.c
+++ b/drivers/staging/wlan-ng/cfg80211.c
@@ -373,7 +373,8 @@ static int prism2_scan(struct wiphy *wiphy,
msg2.beaconperiod.data,
ie_buf,
ie_len,
- (msg2.signal.data - 65536) * 100, /* Conversion to
signed type */
+ /*Conversion to signed type*/
+ (msg2.signal.data - 65536) * 100,
GFP_KERNEL
);
@@ -410,7 +411,7 @@ static int prism2_set_wiphy_params(struct wiphy *wiphy, u32
changed)
data = wiphy->rts_threshold;
result = prism2_domibset_uint32(wlandev,
-
DIDMIB_DOT11MAC_OPERATIONTABLE_RTSTHRESHOLD,
+ DIDMIB_DOT11MAC_OPERATIONTABLE_RTSTHRESHOLD,
data);
if (result) {
err = -EFAULT;
@@ -425,7 +426,7 @@ static int prism2_set_wiphy_params(struct wiphy *wiphy, u32
changed)
data = wiphy->frag_threshold;
result = prism2_domibset_uint32(wlandev,
-
DIDMIB_DOT11MAC_OPERATIONTABLE_FRAGMENTATIONTHRESHOLD,
+ DIDMIB_DOT11MAC_OPERATIONTABLE_FRAGMENTATIONTHRESHOLD,
data);
if (result) {
err = -EFAULT;
@@ -455,7 +456,7 @@ static int prism2_connect(struct wiphy *wiphy, struct
net_device *dev,
if (channel) {
chan = ieee80211_frequency_to_channel(channel->center_freq);
result = prism2_domibset_uint32(wlandev,
-
DIDMIB_DOT11PHY_DSSSTABLE_CURRENTCHANNEL,
+ DIDMIB_DOT11PHY_DSSSTABLE_CURRENTCHANNEL,
chan);
if (result)
goto exit;
@@ -502,13 +503,13 @@ static int prism2_connect(struct wiphy *wiphy, struct
net_device *dev,
* seems reasonable anyways
*/
result = prism2_domibset_uint32(wlandev,
-
DIDMIB_DOT11SMT_PRIVACYTABLE_PRIVACYINVOKED,
+ DIDMIB_DOT11SMT_PRIVACYTABLE_PRIVACYINVOKED,
P80211ENUM_truth_true);
if (result)
goto exit;
result = prism2_domibset_uint32(wlandev,
-
DIDMIB_DOT11SMT_PRIVACYTABLE_EXCLUDEUNENCRYPTED,
+ DIDMIB_DOT11SMT_PRIVACYTABLE_EXCLUDEUNENCRYPTED,
P80211ENUM_truth_true);
if (result)
goto exit;
@@ -518,13 +519,13 @@ static int prism2_connect(struct wiphy *wiphy, struct
net_device *dev,
* and exclude unencrypted
*/
result = prism2_domibset_uint32(wlandev,
-
DIDMIB_DOT11SMT_PRIVACYTABLE_PRIVACYINVOKED,
+ DIDMIB_DOT11SMT_PRIVACYTABLE_PRIVACYINVOKED,
P80211ENUM_truth_false);
if (result)
goto exit;
result = prism2_domibset_uint32(wlandev,
-
DIDMIB_DOT11SMT_PRIVACYTABLE_EXCLUDEUNENCRYPTED,
+ DIDMIB_DOT11SMT_PRIVACYTABLE_EXCLUDEUNENCRYPTED,
P80211ENUM_truth_false);
if (result)
goto exit;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: patch "staging: erofs: keep corrupted fs from crashing kernel in" added to staging-linus
On 2019/1/30 23:05, Gao Xiang wrote:
>
> Hi Greg,
>
> Dan raised some suggestions to me. And I want to get some review ideas from
> Chao...
> Current EROFS works good for the normal image, this patch is used for
> corrupted image only...
>
> Could you kindly drop this patch temporarily and I want to get them
> reviewed...
> Not soon... Thanks!
It seems this patch is the top of staging-linus...Could you kindly drop it and
it is better to get "Reviewed-by:" tag for all EROFS bugfix patches from
corresponding
people (Chao, Dan, or Al if possible) first... Thank you very much!
sorry to trouble you...
Thanks,
Gao Xiang
>
> Thanks,
> Gao Xiang
>
> On 2019/1/30 22:33, gre...@linuxfoundation.org wrote:
>>
>> This is a note to let you know that I've just added the patch titled
>>
>> staging: erofs: keep corrupted fs from crashing kernel in
>>
>> to my staging git tree which can be found at
>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
>> in the staging-linus branch.
>>
>> The patch will show up in the next release of the linux-next tree
>> (usually sometime within the next 24 hours during the week.)
>>
>> The patch will hopefully also be merged in Linus's tree for the
>> next -rc kernel release.
>>
>> If you have any questions about this process, please let me know.
>>
>>
>> From d4104c5e783f5d053b97268fb92001d785de7dd5 Mon Sep 17 00:00:00 2001
>> From: Gao Xiang
>> Date: Tue, 29 Jan 2019 23:55:40 +0800
>> Subject: staging: erofs: keep corrupted fs from crashing kernel in
>> erofs_namei()
>>
>> As Al pointed out, "
>> ... and while we are at it, what happens to
>> unsigned int nameoff = le16_to_cpu(de[mid].nameoff);
>> unsigned int matched = min(startprfx, endprfx);
>>
>> struct qstr dname = QSTR_INIT(data + nameoff,
>> unlikely(mid >= ndirents - 1) ?
>> maxsize - nameoff :
>> le16_to_cpu(de[mid + 1].nameoff) - nameoff);
>>
>> /* string comparison without already matched prefix */
>> int ret = dirnamecmp(name, , );
>> if le16_to_cpu(de[...].nameoff) is not monotonically increasing? I.e.
>> what's to prevent e.g. (unsigned)-1 ending up in dname.len?
>>
>> Corrupted fs image shouldn't oops the kernel.. "
>>
>> Revisit the related lookup flow to address the issue.
>>
>> Fixes: d72d1ce60174 ("staging: erofs: add namei functions")
>> Cc: # 4.19+
>> Suggested-by: Al Viro
>> Signed-off-by: Gao Xiang
>> Signed-off-by: Greg Kroah-Hartman
>> ---
>> drivers/staging/erofs/namei.c | 167 ++
>> 1 file changed, 89 insertions(+), 78 deletions(-)
>>
>> diff --git a/drivers/staging/erofs/namei.c b/drivers/staging/erofs/namei.c
>> index 5596c52e246d..a1300c420e63 100644
>> --- a/drivers/staging/erofs/namei.c
>> +++ b/drivers/staging/erofs/namei.c
>> @@ -15,74 +15,76 @@
>>
>> #include
>>
>> -/* based on the value of qn->len is accurate */
>> -static inline int dirnamecmp(struct qstr *qn,
>> -struct qstr *qd, unsigned int *matched)
>> +struct erofs_qstr {
>> +const unsigned char *name;
>> +const unsigned char *end;
>> +};
>> +
>> +/* based on the end of qn is accurate and it must have the trailing '\0' */
>> +static inline int dirnamecmp(const struct erofs_qstr *qn,
>> + const struct erofs_qstr *qd,
>> + unsigned int *matched)
>> {
>> -unsigned int i = *matched, len = min(qn->len, qd->len);
>> -loop:
>> -if (unlikely(i >= len)) {
>> -*matched = i;
>> -if (qn->len < qd->len) {
>> -/*
>> - * actually (qn->len == qd->len)
>> - * when qd->name[i] == '\0'
>> - */
>> -return qd->name[i] == '\0' ? 0 : -1;
>> +unsigned int i = *matched;
>> +
>> +/*
>> + * on-disk error, let's only BUG_ON in the debugging mode.
>> + * otherwise, it will return 1 to just skip the invalid name
>> + * and go on (in consideration of the lookup performance).
>> + */
>> +DBG_BUGON(qd->name > qd->end);
>> +
>> +/* qd could not have trailing '\0' */
>> +/* However it is absolutely safe if < qd->end */
>> +while (qd->name + i < qd->end && qd->name[i] != '\0') {
>> +if (qn->name[i] != qd->name[i]) {
>> +*matched = i;
>> +return qn->name[i] > qd->name[i] ? 1 : -1;
>> }
>> -return (qn->len > qd->len);
>> +++i;
>> }
>> -
>> -if (qn->name[i] != qd->name[i]) {
>> -*matched = i;
>> -return qn->name[i] > qd->name[i] ? 1 : -1;
>> -}
>> -
>> -++i;
>> -goto loop;
>> +*matched = i;
>> +/* See comments in __d_alloc on the terminating NUL character */
>> +return qn->name[i] == '\0' ? 0 : 1;
>> }
>>
>> -static struct erofs_dirent *find_target_dirent(
>> -struct qstr *name,
>> -u8 *data, int maxsize)
>> +#define
Re: [PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier
On Wed, Jan 30, 2019 at 02:48:27PM +, Carlos Henrique Lima Melara wrote: > This patch fix the checkpatch.p1 warning: > > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 > +/* Why did you not fix up the indentation that was already mentioned? > It includes the SPDX expression for GPL-2.0 and corrects the comment > format to suit the kernel's coding style. Always properly line-wrap your comments. > > Signed-off-by: Carlos (Charles) Henrique Lima Melara > This needs to match your "From:" line. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: erofs: keep corrupted fs from crashing kernel in erofs_namei()
Hi Dan,
Thanks for your kindly review.
On 2019/1/30 22:45, Dan Carpenter wrote:
> On Tue, Jan 29, 2019 at 11:55:40PM +0800, Gao Xiang wrote:
>> +static struct page *find_target_block_classic(struct inode *dir,
>> + struct erofs_qstr *name,
>> + int *_diff,
>> + int *_ndirents)
>> {
>> unsigned int startprfx, endprfx;
>> -unsigned int head, back;
>> +int head, back;
>> struct address_space *const mapping = dir->i_mapping;
>> struct page *candidate = ERR_PTR(-ENOENT);
>>
>> @@ -105,33 +108,34 @@ static struct page *find_target_block_classic(
>> back = inode_datablocks(dir) - 1;
>>
>> while (head <= back) {
>> -unsigned int mid = head + (back - head) / 2;
>> +const int mid = head + (back - head) / 2;
>> struct page *page = read_mapping_page(mapping, mid, NULL);
>>
>> -if (IS_ERR(page)) {
>> -exact_out:
>> -if (!IS_ERR(candidate)) /* valid candidate */
>> -put_page(candidate);
>> -return page;
>> -} else {
>> -int diff;
>> -unsigned int ndirents, matched;
>> -struct qstr dname;
>> +if (!IS_ERR(page)) {
>
> It's almost always better to do failure handling instead of success
> handing because it lets you pull everything in one indent level. You'd
> need to move a bunch of the declarations around.
I just want to leave definition and the initial assignment in one line...
>> struct erofs_dirent *de = kmap_atomic(page);
>> -unsigned int nameoff = le16_to_cpu(de->nameoff);
>> -
>> -ndirents = nameoff / sizeof(*de);
>> +const int nameoff = nameoff_from_disk(de->nameoff,
>> + EROFS_BLKSIZ);
>> +const int ndirents = nameoff / sizeof(*de);
or I have to
unsigned int mid = head + (back - head) / 2;
const int mid = head + (back - head) / 2;
struct page *page = read_mapping_page(mapping, mid, NULL);
struct erofs_dirent *de;
...
int ndirents;
if (IS_ERR(page)) {
...
}
de = kmap_atomic(page);
...
ndirents = nameoff / sizeof(*de);
which takes extra lines...
>
> if (IS_ERR(page))
> goto out;
>
> But really the out label is not part of the loop so you could move it
> to the bottom of the function...
It seems that the out label is the part of loop...
>
>> struct erofs_dirent *de = kmap_atomic(page);
>> -unsigned int nameoff = le16_to_cpu(de->nameoff);
>> -
>> -ndirents = nameoff / sizeof(*de);
>> +const int nameoff = nameoff_from_disk(de->nameoff,
>> + EROFS_BLKSIZ);
>> +const int ndirents = nameoff / sizeof(*de);
>> +int diff;
>> +unsigned int matched;
>> +struct erofs_qstr dname;
>>
>> -/* corrupted dir (should have one entry at least) */
>> -BUG_ON(!ndirents || nameoff > PAGE_SIZE);
>> +if (unlikely(!ndirents)) {
>> +DBG_BUGON(1);
>> +put_page(page);
>> +page = ERR_PTR(-EIO);
>> +goto out;
>
> We need to kunmap_atomic(de) on this path.
Thanks, will fix in the next version...
>
>> +}
>>
>> matched = min(startprfx, endprfx);
>>
>> dname.name = (u8 *)de + nameoff;
>> -dname.len = ndirents == 1 ?
>> -/* since the rest of the last page is 0 */
>> -EROFS_BLKSIZ - nameoff
>> -: le16_to_cpu(de[1].nameoff) - nameoff;
>> +if (ndirents == 1)
>> +dname.end = (u8 *)de + EROFS_BLKSIZ;
>> +else
>> +dname.end = (u8 *)de +
>> +nameoff_from_disk(de[1].nameoff,
>> + EROFS_BLKSIZ);
>>
>> /* string comparison without already matched prefix */
>> diff = dirnamecmp(name, , );
>> @@ -139,7 +143,7 @@ static struct page *find_target_block_classic(
>>
>> if (unlikely(!diff)) {
>> *_diff = 0;
>> -goto exact_out;
>> +goto
[PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier
This patch fix the checkpatch.p1 warning: WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 +/* It includes the SPDX expression for GPL-2.0 and corrects the comment format to suit the kernel's coding style. Signed-off-by: Carlos (Charles) Henrique Lima Melara --- drivers/staging/mt7621-eth/ethtool.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/staging/mt7621-eth/ethtool.c b/drivers/staging/mt7621-eth/ethtool.c index 40a7d47be913..49d417de64c8 100644 --- a/drivers/staging/mt7621-eth/ethtool.c +++ b/drivers/staging/mt7621-eth/ethtool.c @@ -1,15 +1,17 @@ -/* This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License +// SPDX-License-Identifier: GPL-2.0 +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. * - * Copyright (C) 2009-2016 John Crispin - * Copyright (C) 2009-2016 Felix Fietkau - * Copyright (C) 2013-2016 Michael Lee + * Copyright (C) 2009-2016 John Crispin + * Copyright (C) 2009-2016 Felix Fietkau + * Copyright (C) 2013-2016 Michael Lee */ #include "mtk_eth_soc.h" -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: erofs: keep corrupted fs from crashing kernel in erofs_namei()
On Tue, Jan 29, 2019 at 11:55:40PM +0800, Gao Xiang wrote:
> +static struct page *find_target_block_classic(struct inode *dir,
> + struct erofs_qstr *name,
> + int *_diff,
> + int *_ndirents)
> {
> unsigned int startprfx, endprfx;
> - unsigned int head, back;
> + int head, back;
> struct address_space *const mapping = dir->i_mapping;
> struct page *candidate = ERR_PTR(-ENOENT);
>
> @@ -105,33 +108,34 @@ static struct page *find_target_block_classic(
> back = inode_datablocks(dir) - 1;
>
> while (head <= back) {
> - unsigned int mid = head + (back - head) / 2;
> + const int mid = head + (back - head) / 2;
> struct page *page = read_mapping_page(mapping, mid, NULL);
>
> - if (IS_ERR(page)) {
> -exact_out:
> - if (!IS_ERR(candidate)) /* valid candidate */
> - put_page(candidate);
> - return page;
> - } else {
> - int diff;
> - unsigned int ndirents, matched;
> - struct qstr dname;
> + if (!IS_ERR(page)) {
It's almost always better to do failure handling instead of success
handing because it lets you pull everything in one indent level. You'd
need to move a bunch of the declarations around.
if (IS_ERR(page))
goto out;
But really the out label is not part of the loop so you could move it
to the bottom of the function...
> struct erofs_dirent *de = kmap_atomic(page);
> - unsigned int nameoff = le16_to_cpu(de->nameoff);
> -
> - ndirents = nameoff / sizeof(*de);
> + const int nameoff = nameoff_from_disk(de->nameoff,
> + EROFS_BLKSIZ);
> + const int ndirents = nameoff / sizeof(*de);
> + int diff;
> + unsigned int matched;
> + struct erofs_qstr dname;
>
> - /* corrupted dir (should have one entry at least) */
> - BUG_ON(!ndirents || nameoff > PAGE_SIZE);
> + if (unlikely(!ndirents)) {
> + DBG_BUGON(1);
> + put_page(page);
> + page = ERR_PTR(-EIO);
> + goto out;
We need to kunmap_atomic(de) on this path.
> + }
>
> matched = min(startprfx, endprfx);
>
> dname.name = (u8 *)de + nameoff;
> - dname.len = ndirents == 1 ?
> - /* since the rest of the last page is 0 */
> - EROFS_BLKSIZ - nameoff
> - : le16_to_cpu(de[1].nameoff) - nameoff;
> + if (ndirents == 1)
> + dname.end = (u8 *)de + EROFS_BLKSIZ;
> + else
> + dname.end = (u8 *)de +
> + nameoff_from_disk(de[1].nameoff,
> + EROFS_BLKSIZ);
>
> /* string comparison without already matched prefix */
> diff = dirnamecmp(name, , );
> @@ -139,7 +143,7 @@ static struct page *find_target_block_classic(
>
> if (unlikely(!diff)) {
> *_diff = 0;
> - goto exact_out;
> + goto out;
> } else if (diff > 0) {
> head = mid + 1;
> startprfx = matched;
> @@ -147,35 +151,42 @@ static struct page *find_target_block_classic(
> if (likely(!IS_ERR(candidate)))
^^
Not related to the this patch, but I wonder how this works. IS_ERR()
already has an opposite unlikely() inside so I wonder which trumps the
other?
> put_page(candidate);
> candidate = page;
> + *_ndirents = ndirents;
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: android: ion: Restrict cache maintenance to dma mapped memory
On 1/29/19 5:44 PM, Liam Mark wrote:
> On Fri, 18 Jan 2019, Liam Mark wrote:
>
>> On Fri, 18 Jan 2019, Andrew F. Davis wrote:
>>
>>> On 1/18/19 12:37 PM, Liam Mark wrote:
The ION begin_cpu_access and end_cpu_access functions use the
dma_sync_sg_for_cpu and dma_sync_sg_for_device APIs to perform cache
maintenance.
Currently it is possible to apply cache maintenance, via the
begin_cpu_access and end_cpu_access APIs, to ION buffers which are not
dma mapped.
The dma sync sg APIs should not be called on sg lists which have not been
dma mapped as this can result in cache maintenance being applied to the
wrong address. If an sg list has not been dma mapped then its dma_address
field has not been populated, some dma ops such as the swiotlb_dma_ops ops
use the dma_address field to calculate the address onto which to apply
cache maintenance.
Also I don’t think we want CMOs to be applied to a buffer which is not
dma mapped as the memory should already be coherent for access from the
CPU. Any CMOs required for device access taken care of in the
dma_buf_map_attachment and dma_buf_unmap_attachment calls.
So really it only makes sense for begin_cpu_access and end_cpu_access to
apply CMOs if the buffer is dma mapped.
Fix the ION begin_cpu_access and end_cpu_access functions to only apply
cache maintenance to buffers which are dma mapped.
Fixes: 2a55e7b5e544 ("staging: android: ion: Call dma_map_sg for syncing
and mapping")
Signed-off-by: Liam Mark
---
drivers/staging/android/ion/ion.c | 26 +-
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 6f5afab7c1a1..1fe633a7fdba 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -210,6 +210,7 @@ struct ion_dma_buf_attachment {
struct device *dev;
struct sg_table *table;
struct list_head list;
+ bool dma_mapped;
};
static int ion_dma_buf_attach(struct dma_buf *dmabuf,
@@ -231,6 +232,7 @@ static int ion_dma_buf_attach(struct dma_buf *dmabuf,
a->table = table;
a->dev = attachment->dev;
+ a->dma_mapped = false;
INIT_LIST_HEAD(>list);
attachment->priv = a;
@@ -261,12 +263,18 @@ static struct sg_table *ion_map_dma_buf(struct
dma_buf_attachment *attachment,
{
struct ion_dma_buf_attachment *a = attachment->priv;
struct sg_table *table;
+ struct ion_buffer *buffer = attachment->dmabuf->priv;
table = a->table;
+ mutex_lock(>lock);
if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
- direction))
+ direction)) {
+ mutex_unlock(>lock);
return ERR_PTR(-ENOMEM);
+ }
+ a->dma_mapped = true;
+ mutex_unlock(>lock);
return table;
}
@@ -275,7 +283,13 @@ static void ion_unmap_dma_buf(struct
dma_buf_attachment *attachment,
struct sg_table *table,
enum dma_data_direction direction)
{
+ struct ion_dma_buf_attachment *a = attachment->priv;
+ struct ion_buffer *buffer = attachment->dmabuf->priv;
+
+ mutex_lock(>lock);
dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
+ a->dma_mapped = false;
+ mutex_unlock(>lock);
}
static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
@@ -346,8 +360,9 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf
*dmabuf,
mutex_lock(>lock);
list_for_each_entry(a, >attachments, list) {
>>>
>>> When no devices are attached then buffer->attachments is empty and the
>>> below does not run, so if I understand this patch correctly then what
>>> you are protecting against is CPU access in the window after
>>> dma_buf_attach but before dma_buf_map.
>>>
>>
>> Yes
>>
>>> This is the kind of thing that again makes me think a couple more
>>> ordering requirements on DMA-BUF ops are needed. DMA-BUFs do not require
>>> the backing memory to be allocated until map time, this is why the
>>> dma_address field would still be null as you note in the commit message.
>>> So why should the CPU be performing accesses on a buffer that is not
>>> actually backed yet?
>>>
>>> I can think of two solutions:
>>>
>>> 1) Only allow CPU access (mmap, kmap, {begin,end}_cpu_access) while at
>>> least one device is mapped.
>>>
>>
>> Would be quite limiting to clients.
>>
I can agree with that, option two seems more reasonable.
>>> 2) Treat the CPU access request like the a device map request and
>>> trigger the allocation of backing memory just like if a device map had
Re: [PATCH] MIPS: Select PINCTRL_RT2880 when RALINK is enabled
On Tue, Jan 29, 2019 at 08:09:07PM +, Paul Burton wrote: > Hi Nishad, > > On Tue, Jan 29, 2019 at 08:55:27PM +0530, Nishad Kamdar wrote: > > This patch selects config PINCTRL_RT2880 when config RALINK is > > enabled as per drivers/staging/mt7621-pinctrl/TODO list. PINCTRL > > is also selected when RALINK is enabled to avoid config dependency > > warnings. > > > > Signed-off-by: Nishad Kamdar > > --- > > arch/mips/Kconfig | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > > index e2fc88da0223..cea529cf6284 100644 > > --- a/arch/mips/Kconfig > > +++ b/arch/mips/Kconfig > > @@ -623,6 +623,8 @@ config RALINK > > select CLKDEV_LOOKUP > > select ARCH_HAS_RESET_CONTROLLER > > select RESET_CONTROLLER > > + select PINCTRL > > + select PINCTRL_RT2880 > > > > config SGI_IP22 > > bool "SGI IP22 (Indy/Indigo2)" > > Wouldn't this also require selecting STAGING? Perhaps that's why it > wasn't done in the first place? > > Thanks, > Paul Ok, got it. Thanks for the review. Regards, Nishad ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] binderfs: remove separate device_initcall()
On Wed, Jan 23, 2019 at 12:41:16PM +0100, Christian Brauner wrote:
> binderfs should not have a separate device_initcall(). When a kernel is
> compiled with CONFIG_ANDROID_BINDERFS register the filesystem alongside
> CONFIG_ANDROID_IPC. This use-case is especially sensible when users specify
> CONFIG_ANDROID_IPC=y, CONFIG_ANDROID_BINDERFS=y and
> ANDROID_BINDER_DEVICES="".
> When CONFIG_ANDROID_BINDERFS=n then this always succeeds so there's no
> regression potential for legacy workloads.
>
> Signed-off-by: Christian Brauner
> ---
> drivers/android/binder.c | 4
> drivers/android/binder_internal.h | 9 +
> drivers/android/binderfs.c| 4 +---
> 3 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index cdfc87629efb..751d76173f81 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5915,6 +5915,10 @@ static int __init binder_init(void)
> goto err_init_binder_device_failed;
> }
>
> + ret = init_binderfs();
> + if (ret)
> + goto err_init_binder_device_failed;
> +
> return ret;
>
> err_init_binder_device_failed:
> diff --git a/drivers/android/binder_internal.h
> b/drivers/android/binder_internal.h
> index 7fb97f503ef2..045b3e42d98b 100644
> --- a/drivers/android/binder_internal.h
> +++ b/drivers/android/binder_internal.h
> @@ -46,4 +46,13 @@ static inline bool is_binderfs_device(const struct inode
> *inode)
> }
> #endif
>
> +#ifdef CONFIG_ANDROID_BINDERFS
> +extern int __init init_binderfs(void);
> +#else
> +static inline int __init init_binderfs(void)
> +{
> + return 0;
> +}
> +#endif
> +
> #endif /* _LINUX_BINDER_INTERNAL_H */
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index 7a550104a722..e773f45d19d9 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -550,7 +550,7 @@ static struct file_system_type binder_fs_type = {
> .fs_flags = FS_USERNS_MOUNT,
> };
>
> -static int __init init_binderfs(void)
> +int __init init_binderfs(void)
> {
> int ret;
>
> @@ -568,5 +568,3 @@ static int __init init_binderfs(void)
>
> return ret;
> }
> -
> -device_initcall(init_binderfs);
I get a build warning when applying this patch :(
Please fix up and resend.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v11 05/13] media: staging/imx7: add MIPI CSI-2 receiver subdev for i.MX7
Hi Rui, On Thu, Jan 24, 2019 at 04:09:20PM +, Rui Miguel Silva wrote: > Adds MIPI CSI-2 subdev for i.MX7 to connect with sensors with a MIPI > CSI-2 interface. > > Signed-off-by: Rui Miguel Silva > --- > drivers/staging/media/imx/Makefile |1 + > drivers/staging/media/imx/imx7-mipi-csis.c | 1184 > 2 files changed, 1185 insertions(+) > create mode 100644 drivers/staging/media/imx/imx7-mipi-csis.c > > diff --git a/drivers/staging/media/imx/Makefile > b/drivers/staging/media/imx/Makefile > index 074f016d3519..d2d909a36239 100644 > --- a/drivers/staging/media/imx/Makefile > +++ b/drivers/staging/media/imx/Makefile > @@ -14,3 +14,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o > obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o > > obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o > +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c > b/drivers/staging/media/imx/imx7-mipi-csis.c > new file mode 100644 > index ..2d54fc7b20a0 > --- /dev/null > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > @@ -0,0 +1,1184 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Freescale i.MX7 SoC series MIPI-CSI V3.3 receiver driver > + * > + * Copyright (C) 2019 Linaro Ltd > + * Copyright (C) 2015-2016 Freescale Semiconductor, Inc. All Rights Reserved. > + * Copyright (C) 2011 - 2013 Samsung Electronics Co., Ltd. > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > +#include "imx-media.h" > + > +#define CSIS_DRIVER_NAME "imx7-mipi-csis" > +#define CSIS_SUBDEV_NAME CSIS_DRIVER_NAME > + > +#define CSIS_PAD_SINK0 > +#define CSIS_PAD_SOURCE 1 > +#define CSIS_PADS_NUM2 > + > +#define MIPI_CSIS_DEF_PIX_WIDTH 640 > +#define MIPI_CSIS_DEF_PIX_HEIGHT 480 > + > +/* Register map definition */ > + > +/* CSIS common control */ > +#define MIPI_CSIS_CMN_CTRL 0x04 > +#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW BIT(16) > +#define MIPI_CSIS_CMN_CTRL_INTER_MODEBIT(10) > +#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRLBIT(2) > +#define MIPI_CSIS_CMN_CTRL_RESET BIT(1) > +#define MIPI_CSIS_CMN_CTRL_ENABLEBIT(0) > + > +#define MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET8 > +#define MIPI_CSIS_CMN_CTRL_LANE_NR_MASK (3 << 8) > + > +/* CSIS clock control */ > +#define MIPI_CSIS_CLK_CTRL 0x08 > +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH3(x) ((x) << 28) > +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH2(x) ((x) << 24) > +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH1(x) ((x) << 20) > +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(x) ((x) << 16) > +#define MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MSK(0xf << 4) > +#define MIPI_CSIS_CLK_CTRL_WCLK_SRC BIT(0) > + > +/* CSIS Interrupt mask */ > +#define MIPI_CSIS_INTMSK 0x10 > +#define MIPI_CSIS_INTMSK_EVEN_BEFORE BIT(31) > +#define MIPI_CSIS_INTMSK_EVEN_AFTER BIT(30) > +#define MIPI_CSIS_INTMSK_ODD_BEFORE BIT(29) > +#define MIPI_CSIS_INTMSK_ODD_AFTER BIT(28) > +#define MIPI_CSIS_INTMSK_FRAME_START BIT(24) > +#define MIPI_CSIS_INTMSK_FRAME_END BIT(20) > +#define MIPI_CSIS_INTMSK_ERR_SOT_HS BIT(16) > +#define MIPI_CSIS_INTMSK_ERR_LOST_FS BIT(12) > +#define MIPI_CSIS_INTMSK_ERR_LOST_FE BIT(8) > +#define MIPI_CSIS_INTMSK_ERR_OVERBIT(4) > +#define MIPI_CSIS_INTMSK_ERR_WRONG_CFG BIT(3) > +#define MIPI_CSIS_INTMSK_ERR_ECC BIT(2) > +#define MIPI_CSIS_INTMSK_ERR_CRC BIT(1) > +#define MIPI_CSIS_INTMSK_ERR_UNKNOWN BIT(0) > + > +/* CSIS Interrupt source */ > +#define MIPI_CSIS_INTSRC 0x14 > +#define MIPI_CSIS_INTSRC_EVEN_BEFORE BIT(31) > +#define MIPI_CSIS_INTSRC_EVEN_AFTER BIT(30) > +#define MIPI_CSIS_INTSRC_EVENBIT(30) > +#define MIPI_CSIS_INTSRC_ODD_BEFORE BIT(29) > +#define MIPI_CSIS_INTSRC_ODD_AFTER BIT(28) > +#define MIPI_CSIS_INTSRC_ODD (0x3 << 28) > +#define MIPI_CSIS_INTSRC_NON_IMAGE_DATA (0xf << 28) > +#define MIPI_CSIS_INTSRC_FRAME_START BIT(24) > +#define MIPI_CSIS_INTSRC_FRAME_END BIT(20) > +#define MIPI_CSIS_INTSRC_ERR_SOT_HS BIT(16) > +#define MIPI_CSIS_INTSRC_ERR_LOST_FS BIT(12) > +#define MIPI_CSIS_INTSRC_ERR_LOST_FE BIT(8) > +#define MIPI_CSIS_INTSRC_ERR_OVERBIT(4) > +#define MIPI_CSIS_INTSRC_ERR_WRONG_CFG BIT(3) > +#define MIPI_CSIS_INTSRC_ERR_ECC BIT(2) > +#define MIPI_CSIS_INTSRC_ERR_CRC BIT(1) > +#define MIPI_CSIS_INTSRC_ERR_UNKNOWN BIT(0) > +#define MIPI_CSIS_INTSRC_ERRORS 0xf > + > +/* D-PHY status control */ > +#define MIPI_CSIS_DPHYSTATUS 0x20 > +#define MIPI_CSIS_DPHYSTATUS_ULPS_DATBIT(8) > +#define MIPI_CSIS_DPHYSTATUS_STOPSTATE_DAT BIT(4) >
Re: [PATCH 2/4] staging: android: ion: Restrict cache maintenance to dma mapped memory
Hi Liam,
On Tue, Jan 29, 2019 at 03:44:53PM -0800, Liam Mark wrote:
> On Fri, 18 Jan 2019, Liam Mark wrote:
>
> > On Fri, 18 Jan 2019, Andrew F. Davis wrote:
> >
> > > On 1/18/19 12:37 PM, Liam Mark wrote:
> > > > The ION begin_cpu_access and end_cpu_access functions use the
> > > > dma_sync_sg_for_cpu and dma_sync_sg_for_device APIs to perform cache
> > > > maintenance.
> > > >
> > > > Currently it is possible to apply cache maintenance, via the
> > > > begin_cpu_access and end_cpu_access APIs, to ION buffers which are not
> > > > dma mapped.
> > > >
> > > > The dma sync sg APIs should not be called on sg lists which have not
> > > > been
> > > > dma mapped as this can result in cache maintenance being applied to the
> > > > wrong address. If an sg list has not been dma mapped then its
> > > > dma_address
> > > > field has not been populated, some dma ops such as the swiotlb_dma_ops
> > > > ops
> > > > use the dma_address field to calculate the address onto which to apply
> > > > cache maintenance.
> > > >
> > > > Also I don’t think we want CMOs to be applied to a buffer which is not
> > > > dma mapped as the memory should already be coherent for access from the
> > > > CPU. Any CMOs required for device access taken care of in the
> > > > dma_buf_map_attachment and dma_buf_unmap_attachment calls.
> > > > So really it only makes sense for begin_cpu_access and end_cpu_access to
> > > > apply CMOs if the buffer is dma mapped.
> > > >
> > > > Fix the ION begin_cpu_access and end_cpu_access functions to only apply
> > > > cache maintenance to buffers which are dma mapped.
> > > >
> > > > Fixes: 2a55e7b5e544 ("staging: android: ion: Call dma_map_sg for
> > > > syncing and mapping")
> > > > Signed-off-by: Liam Mark
> > > > ---
> > > > drivers/staging/android/ion/ion.c | 26 +-
> > > > 1 file changed, 21 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/android/ion/ion.c
> > > > b/drivers/staging/android/ion/ion.c
> > > > index 6f5afab7c1a1..1fe633a7fdba 100644
> > > > --- a/drivers/staging/android/ion/ion.c
> > > > +++ b/drivers/staging/android/ion/ion.c
> > > > @@ -210,6 +210,7 @@ struct ion_dma_buf_attachment {
> > > > struct device *dev;
> > > > struct sg_table *table;
> > > > struct list_head list;
> > > > + bool dma_mapped;
> > > > };
> > > >
> > > > static int ion_dma_buf_attach(struct dma_buf *dmabuf,
> > > > @@ -231,6 +232,7 @@ static int ion_dma_buf_attach(struct dma_buf
> > > > *dmabuf,
> > > >
> > > > a->table = table;
> > > > a->dev = attachment->dev;
> > > > + a->dma_mapped = false;
> > > > INIT_LIST_HEAD(>list);
> > > >
> > > > attachment->priv = a;
> > > > @@ -261,12 +263,18 @@ static struct sg_table *ion_map_dma_buf(struct
> > > > dma_buf_attachment *attachment,
> > > > {
> > > > struct ion_dma_buf_attachment *a = attachment->priv;
> > > > struct sg_table *table;
> > > > + struct ion_buffer *buffer = attachment->dmabuf->priv;
> > > >
> > > > table = a->table;
> > > >
> > > > + mutex_lock(>lock);
> > > > if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
> > > > - direction))
> > > > + direction)) {
> > > > + mutex_unlock(>lock);
> > > > return ERR_PTR(-ENOMEM);
> > > > + }
> > > > + a->dma_mapped = true;
> > > > + mutex_unlock(>lock);
> > > >
> > > > return table;
> > > > }
> > > > @@ -275,7 +283,13 @@ static void ion_unmap_dma_buf(struct
> > > > dma_buf_attachment *attachment,
> > > > struct sg_table *table,
> > > > enum dma_data_direction direction)
> > > > {
> > > > + struct ion_dma_buf_attachment *a = attachment->priv;
> > > > + struct ion_buffer *buffer = attachment->dmabuf->priv;
> > > > +
> > > > + mutex_lock(>lock);
> > > > dma_unmap_sg(attachment->dev, table->sgl, table->nents,
> > > > direction);
> > > > + a->dma_mapped = false;
> > > > + mutex_unlock(>lock);
> > > > }
> > > >
> > > > static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
> > > > @@ -346,8 +360,9 @@ static int ion_dma_buf_begin_cpu_access(struct
> > > > dma_buf *dmabuf,
> > > >
> > > > mutex_lock(>lock);
> > > > list_for_each_entry(a, >attachments, list) {
> > >
> > > When no devices are attached then buffer->attachments is empty and the
> > > below does not run, so if I understand this patch correctly then what
> > > you are protecting against is CPU access in the window after
> > > dma_buf_attach but before dma_buf_map.
> > >
> >
> > Yes
> >
> > > This is the kind of thing that again makes me think a couple more
> > > ordering requirements on DMA-BUF ops are needed. DMA-BUFs do not require
> > > the backing memory to be allocated until map time, this is why the
> > >
Re: [PATCH v11 00/13] media: staging/imx7: add i.MX7 media driver
On Thu, Jan 24, 2019 at 04:09:15PM +, Rui Miguel Silva wrote: > Hi, > This series introduces the Media driver to work with the i.MX7 SoC. it uses > the > already existing imx media core drivers but since the i.MX7, contrary to > i.MX5/6, do not have an IPU and because of that some changes in the imx media > core are made along this series to make it support that case. > > This patches adds CSI and MIPI-CSI2 drivers for i.MX7, along with several > configurations changes for this to work as a capture subsystem. Some bugs are > also fixed along the line. And necessary documentation. > > For a more detailed view of the capture paths, pads links in the i.MX7 please > take a look at the documentation in PATCH 10. > > The system used to test and develop this was the Warp7 board with an OV2680 > sensor, which output format is 10-bit bayer. So, only MIPI interface was > tested, a scenario with an parallel input would nice to have. > > Bellow goes an example of the output of the pads and links and the output of > v4l2-compliance testing. > > The v4l-utils version used is: > v4l2-compliance SHA: 1a6c8fe9a65c26e78ba34bd4aa2df28ede7d00cb, 32 bits > > The Media Driver fail some tests but this failures are coming from code out of > scope of this series (imx-capture), and some from the sensor OV2680 > but that I think not related with the sensor driver but with the testing and > core. > > The csi and mipi-csi entities pass all compliance tests. For patches 1, 4 and 10--13: Acked-by: Sakari Ailus -- Sakari Ailus sakari.ai...@linux.intel.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v11 03/13] media: staging/imx7: add imx7 CSI subdev driver
Hi Rui, A few more comments below. On Thu, Jan 24, 2019 at 04:09:18PM +, Rui Miguel Silva wrote: > This add the media entity subdevice and control driver for the i.MX7 > CMOS Sensor Interface. > > Signed-off-by: Rui Miguel Silva > --- > drivers/staging/media/imx/imx7-media-csi.c | 1360 > 1 file changed, 1360 insertions(+) > create mode 100644 drivers/staging/media/imx/imx7-media-csi.c > > diff --git a/drivers/staging/media/imx/imx7-media-csi.c > b/drivers/staging/media/imx/imx7-media-csi.c > new file mode 100644 > index ..44ba008fdc12 > --- /dev/null > +++ b/drivers/staging/media/imx/imx7-media-csi.c > @@ -0,0 +1,1360 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC > + * > + * Copyright (c) 2019 Linaro Ltd > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include "imx-media.h" > + > +#define IMX7_CSI_PAD_SINK0 > +#define IMX7_CSI_PAD_SRC 1 > +#define IMX7_CSI_PADS_NUM2 > + > +/* reset values */ > +#define CSICR1_RESET_VAL 0x4800 > +#define CSICR2_RESET_VAL 0x0 > +#define CSICR3_RESET_VAL 0x0 > + > +/* csi control reg 1 */ > +#define BIT_SWAP16_ENBIT(31) > +#define BIT_EXT_VSYNCBIT(30) > +#define BIT_EOF_INT_EN BIT(29) > +#define BIT_PRP_IF_ENBIT(28) > +#define BIT_CCIR_MODEBIT(27) > +#define BIT_COF_INT_EN BIT(26) > +#define BIT_SF_OR_INTEN BIT(25) > +#define BIT_RF_OR_INTEN BIT(24) > +#define BIT_SFF_DMA_DONE_INTEN BIT(22) > +#define BIT_STATFF_INTEN BIT(21) > +#define BIT_FB2_DMA_DONE_INTEN BIT(20) > +#define BIT_FB1_DMA_DONE_INTEN BIT(19) > +#define BIT_RXFF_INTEN BIT(18) > +#define BIT_SOF_POL BIT(17) > +#define BIT_SOF_INTENBIT(16) > +#define BIT_MCLKDIV (0xF << 12) > +#define BIT_HSYNC_POLBIT(11) > +#define BIT_CCIR_EN BIT(10) > +#define BIT_MCLKEN BIT(9) > +#define BIT_FCC BIT(8) > +#define BIT_PACK_DIR BIT(7) > +#define BIT_CLR_STATFIFO BIT(6) > +#define BIT_CLR_RXFIFO BIT(5) > +#define BIT_GCLK_MODEBIT(4) > +#define BIT_INV_DATA BIT(3) > +#define BIT_INV_PCLK BIT(2) > +#define BIT_REDGEBIT(1) > +#define BIT_PIXEL_BITBIT(0) > + > +#define SHIFT_MCLKDIV12 > + > +/* control reg 3 */ > +#define BIT_FRMCNT (0x << 16) > +#define BIT_FRMCNT_RST BIT(15) > +#define BIT_DMA_REFLASH_RFF BIT(14) > +#define BIT_DMA_REFLASH_SFF BIT(13) > +#define BIT_DMA_REQ_EN_RFF BIT(12) > +#define BIT_DMA_REQ_EN_SFF BIT(11) > +#define BIT_STATFF_LEVEL (0x7 << 8) > +#define BIT_HRESP_ERR_EN BIT(7) > +#define BIT_RXFF_LEVEL (0x7 << 4) > +#define BIT_TWO_8BIT_SENSOR BIT(3) > +#define BIT_ZERO_PACK_EN BIT(2) > +#define BIT_ECC_INT_EN BIT(1) > +#define BIT_ECC_AUTO_EN BIT(0) > + > +#define SHIFT_FRMCNT 16 > +#define SHIFT_RXFIFO_LEVEL 4 > + > +/* csi status reg */ > +#define BIT_ADDR_CH_ERR_INT BIT(28) > +#define BIT_FIELD0_INT BIT(27) > +#define BIT_FIELD1_INT BIT(26) > +#define BIT_SFF_OR_INT BIT(25) > +#define BIT_RFF_OR_INT BIT(24) > +#define BIT_DMA_TSF_DONE_SFF BIT(22) > +#define BIT_STATFF_INT BIT(21) > +#define BIT_DMA_TSF_DONE_FB2 BIT(20) > +#define BIT_DMA_TSF_DONE_FB1 BIT(19) > +#define BIT_RXFF_INT BIT(18) > +#define BIT_EOF_INT BIT(17) > +#define BIT_SOF_INT BIT(16) > +#define BIT_F2_INT BIT(15) > +#define BIT_F1_INT BIT(14) > +#define BIT_COF_INT BIT(13) > +#define BIT_HRESP_ERR_INTBIT(7) > +#define BIT_ECC_INT BIT(1) > +#define BIT_DRDY BIT(0) > + > +/* csi control reg 18 */ > +#define BIT_CSI_HW_ENABLEBIT(31) > +#define BIT_MIPI_DATA_FORMAT_RAW8(0x2a << 25) > +#define BIT_MIPI_DATA_FORMAT_RAW10 (0x2b << 25) > +#define BIT_MIPI_DATA_FORMAT_RAW12 (0x2c << 25) > +#define BIT_MIPI_DATA_FORMAT_RAW14 (0x2d << 25) > +#define BIT_MIPI_DATA_FORMAT_YUV422_8B (0x1e << 25) > +#define BIT_MIPI_DATA_FORMAT_MASK(0x3F << 25) > +#define BIT_MIPI_DATA_FORMAT_OFFSET 25 > +#define BIT_DATA_FROM_MIPI BIT(22) > +#define BIT_MIPI_YU_SWAP BIT(21) > +#define BIT_MIPI_DOUBLE_CMPNTBIT(20) > +#define BIT_BASEADDR_CHG_ERR_EN BIT(9) > +#define BIT_BASEADDR_SWITCH_SEL BIT(5) > +#define BIT_BASEADDR_SWITCH_EN BIT(4) > +#define BIT_PARALLEL24_ENBIT(3) > +#define BIT_DEINTERLACE_EN BIT(2) > +#define
Re: [PATCH v2 1/2] staging: iio: ad7780: add gain & filter gpio support
On Du, 2019-01-27 at 18:30 -0200, Renato Lui Geh wrote:
> Previously, the AD7780 driver only supported gpio for the 'powerdown'
> pin. This commit adds suppport for the 'gain' and 'filter' pin.
>
> Signed-off-by: Renato Lui Geh
> Signed-off-by: Giuliano Belinassi
> Co-developed-by: Giuliano Belinassi
> ---
> Changes in v2:
> - Filter reading changed to mHz
> - Storing filter, gain and voltage to chip_info
Hi,
Comments inline.
Regards,
Stefan
>
> drivers/staging/iio/adc/ad7780.c | 103 ++---
> include/linux/iio/adc/ad_sigma_delta.h | 5 ++
> 2 files changed, 99 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7780.c
> b/drivers/staging/iio/adc/ad7780.c
> index c4a85789c2db..82394e31b168 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -39,6 +39,15 @@
> #define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2)
> #define AD7170_PATTERN_MASK(AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2)
>
> +#define AD7780_GAIN_GPIO 0
> +#define AD7780_FILTER_GPIO 1
> +
> +#define AD7780_GAIN_MIDPOINT 64
> +#define AD7780_FILTER_MIDPOINT 13350
> +
> +static const unsigned int ad778x_gain[2]= { 1, 128 };
> +static const unsigned int ad778x_filter[2] = { 1, 16700 };
I would name this array ad778x_odr_avail or something like that. We should
also consider adding the option to read the available sampling frequencies
from user space, but let's leave this for another commit.
> +
> struct ad7780_chip_info {
> struct iio_chan_specchannel;
> unsigned intpattern_mask;
> @@ -50,7 +59,11 @@ struct ad7780_state {
> const struct ad7780_chip_info *chip_info;
> struct regulator*reg;
> struct gpio_desc*powerdown_gpio;
> - unsigned intgain;
> + struct gpio_desc*gain_gpio;
> + struct gpio_desc*filter_gpio;
> + unsigned intgain;
> + unsigned intfilter;
Also, this could be renamed as odr.
> + unsigned intint_vref_mv;
>
> struct ad_sigma_delta sd;
> };
> @@ -104,17 +117,65 @@ static int ad7780_read_raw(struct iio_dev
> *indio_dev,
> voltage_uv = regulator_get_voltage(st->reg);
> if (voltage_uv < 0)
> return voltage_uv;
> - *val = (voltage_uv / 1000) * st->gain;
> + voltage_uv /= 1000;
> + *val = voltage_uv * st->gain;
> *val2 = chan->scan_type.realbits - 1;
> + st->int_vref_mv = voltage_uv;
> return IIO_VAL_FRACTIONAL_LOG2;
> case IIO_CHAN_INFO_OFFSET:
> *val = -(1 << (chan->scan_type.realbits - 1));
> return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *val = st->filter;
> + return IIO_VAL_INT;
> }
>
> return -EINVAL;
> }
>
> +static int ad7780_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val,
> + int val2,
> + long m)
> +{
> + struct ad7780_state *st = iio_priv(indio_dev);
> + const struct ad7780_chip_info *chip_info = st->chip_info;
> + int vref, gain;
> + unsigned int full_scale;
> +
> + if (!chip_info->is_ad778x)
> + return 0;
> +
> + switch (m) {
> + case IIO_CHAN_INFO_SCALE:
> + if (val != 0)
> + return -EINVAL;
> +
> + vref = st->int_vref_mv * 100LL;
From the datasheet, the VREF is ±5 V, therefore your vref variable will
overflow. You probably need unsigned long long.
> + full_scale = 1 << (chip_info->channel.scane_type.realbis
> - 1);
> + gain = DIV_ROUND_CLOSEST(vref, full_scale);
> + gain = DIV_ROUND_CLOSEST(gain, val2);
> + st->gain = gain;
> + if (gain < AD7780_GAIN_MIDPOINT)
> + gain = 0;
> + else
> + gain = 1;
> + gpiod_set_value(st->gain_gpio, gain);
> + break;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT)
> + val = 0;
> + else
> + val = 1;
> + st->filter = ad778x_filter[val];
> + gpiod_set_value(st->filter_gpio, val);
> + break;
> + }
> +
> + return 0;
> +}
> +
> static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta,
> unsigned int raw_sample)
> {
> @@ -126,10 +187,8 @@ static int ad7780_postprocess_sample(struct
> ad_sigma_delta *sigma_delta,
> return -EIO;
>
> if
Re: [PATCH] tty: Fix WARNING in tty_set_termios
On Mon, Jan 28, 2019 at 02:29:22PM -0700, shuah wrote:
> On 1/25/19 9:14 PM, Al Viro wrote:
> > On Fri, Jan 25, 2019 at 04:29:05PM -0700, Shuah Khan wrote:
> >> tty_set_termios() has the following WARMN_ON which can be triggered with a
> >> syscall to invoke TIOCGETD __NR_ioctl.
You meant TIOCSETD here, and in fact its the call which sets the uart
protocol that triggers the warning.
> >> WARN_ON(tty->driver->type == TTY_DRIVER_TYPE_PTY &&
> >> tty->driver->subtype == PTY_TYPE_MASTER);
> >> Reference:
> >> https://syzkaller.appspot.com/bug?id=2410d22f1d8e5984217329dd0884b01d99e3e48d
> >>
> >> A simple change would have been to print error message instead of WARN_ON.
> >> However, the callers assume that tty_set_termios() always returns 0 and
> >> don't check return value. The complete solution is fixing all the callers
> >> to check error and bail out to fix the WARN_ON.
> >>
> >> This fix changes tty_set_termios() to return error and all the callers
> >> to check error and bail out. The reproducer is used to reproduce the
> >> problem and verify the fix.
> >
> >> --- a/drivers/bluetooth/hci_ldisc.c
> >> +++ b/drivers/bluetooth/hci_ldisc.c
> >> @@ -321,6 +321,8 @@ void hci_uart_set_flow_control(struct hci_uart *hu,
> >> bool enable)
> >>status = tty_set_termios(tty, );
> >>BT_DBG("Disabling hardware flow control: %s",
> >> status ? "failed" : "success");
> >> + if (status)
> >> + return;
> >
> > Can that ldisc end up set on pty master? And does it make any sense there?
>
> The initial objective of the patch is to prevent the WARN_ON by making
> the change to return error instead of WARN_ON. However, without changes
> to places that don't check the return and keep making progress, there
> will be secondary problems.
>
> Without this change to return here, instead of WARN_ON, it will fail
> with the following NULL pointer dereference at the next thing
> hci_uart_set_flow_control() attempts.
>
> status = tty->driver->ops->tiocmget(tty);
>
> kernel: [10140.649783] BUG: unable to handle kernel NULL pointer
That's a separate issue, which is being fixed:
https://lkml.kernel.org/r/20190130095938.GP3691@localhost
> > IOW, I don't believe that this patch makes any sense. If anything,
> > we need to prevent unconditional tty_set_termios() on the path
> > that *does* lead to calling it for pty.
>
> I don't think preventing unconditional tty_set_termios() is enough to
> prevent secondary problems such as the one above.
>
> For example, the following call chain leads to the WARN_ON that was
> reported. Even if void hci_uart_set_baudrate() prevents the very first
> tty_set_termios() call, its caller hci_uart_setup() continues with
> more tty setup. It goes ahead to call driver setup callback. The
> driver callback goes on to do more setup calling tty_set_termios().
>
> WARN_ON call path:
> hci_uart_set_baudrate+0x1cc/0x250 drivers/bluetooth/hci_ldisc.c:378
> hci_uart_setup+0xa2/0x490 drivers/bluetooth/hci_ldisc.c:401
> hci_dev_do_open+0x6b1/0x1920 net/bluetooth/hci_core.c:1423
>
> Once this WARN_ON is changed to return error, the following
> happens, when hci_uart_setup() does driver setup callback.
>
> kernel: [10140.649836] mrvl_setup+0x17/0x80 [hci_uart]
> kernel: [10140.649840] hci_uart_setup+0x56/0x160 [hci_uart]
> kernel: [10140.649850] hci_dev_do_open+0xe6/0x630 [bluetooth]
> kernel: [10140.649860] hci_power_on+0x52/0x220 [bluetooth]
>
> I think continuing to catch the invalid condition in tty_set_termios()
> and preventing progress by checking return value is a straight forward
> change to avoid secondary problems, and it might be difficult to catch
> all the cases where it could fail.
I agree with Al that this change doesn't make much sense. The WARN_ON
is there to catch any bugs leading to the termios being changed for a
master side pty. Those should bugs should be fixed, and not worked
around in order to silence a WARN_ON.
The problem started with 7721383f4199 ("Bluetooth: hci_uart: Support
operational speed during setup") which introduced a new way for how
tty_set_termios() could end up being called for a master pty.
As Al hinted at, setting these ldiscs for a master pty really makes no
sense and perhaps that is what we should prevent unless simply making
sure they do not call tty_set_termios() is sufficient for the time
being.
Finally, note that serdev never operates on a pty, and that this is only
an issue for (the three) line disciplines.
Johan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: speakup: fix tty-operation NULL derefs
Johan Hovold, le mer. 30 janv. 2019 10:49:34 +0100, a ecrit:
> The send_xchar() and tiocmset() tty operations are optional. Add the
> missing sanity checks to prevent user-space triggerable NULL-pointer
> dereferences.
>
> Fixes: 6b9ad1c742bf ("staging: speakup: add send_xchar, tiocmset and input
> functionality for tty")
> Cc: stable # 4.13
> Cc: Okash Khawaja
> Cc: Samuel Thibault
> Signed-off-by: Johan Hovold
Indeed.
Reviewed-by: Samuel Thibault
> ---
> drivers/staging/speakup/spk_ttyio.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/speakup/spk_ttyio.c
> b/drivers/staging/speakup/spk_ttyio.c
> index c92bbd05516e..005de0024dd4 100644
> --- a/drivers/staging/speakup/spk_ttyio.c
> +++ b/drivers/staging/speakup/spk_ttyio.c
> @@ -265,7 +265,8 @@ static void spk_ttyio_send_xchar(char ch)
> return;
> }
>
> - speakup_tty->ops->send_xchar(speakup_tty, ch);
> + if (speakup_tty->ops->send_xchar)
> + speakup_tty->ops->send_xchar(speakup_tty, ch);
> mutex_unlock(_tty_mutex);
> }
>
> @@ -277,7 +278,8 @@ static void spk_ttyio_tiocmset(unsigned int set, unsigned
> int clear)
> return;
> }
>
> - speakup_tty->ops->tiocmset(speakup_tty, set, clear);
> + if (speakup_tty->ops->tiocmset)
> + speakup_tty->ops->tiocmset(speakup_tty, set, clear);
> mutex_unlock(_tty_mutex);
> }
>
> --
> 2.20.1
>
--
Samuel
R: Parce que ça renverse bêtement l'ordre naturel de lecture!
Q: Mais pourquoi citer en fin d'article est-il si effroyable?
R: Citer en fin d'article
Q: Quelle est la chose la plus désagréable sur les groupes de news?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
Sent from my iPad > On Jan 30, 2019, at 3:17 PM, Tomasz Figa wrote: > >> On Wed, Jan 30, 2019 at 3:28 PM Ayaka wrote: >> >> >> >> Sent from my iPad >> >>> On Jan 30, 2019, at 11:35 AM, Tomasz Figa wrote: >>> >>> On Wed, Jan 30, 2019 at 11:29 AM Alexandre Courbot >>> wrote: >> On Wed, Jan 30, 2019 at 6:41 AM Nicolas Dufresne >> wrote: >> >> Le mardi 29 janvier 2019 à 16:44 +0900, Alexandre Courbot a écrit : >> On Fri, Jan 25, 2019 at 10:04 PM Paul Kocialkowski >> wrote: >>> Hi, >>> On Thu, 2019-01-24 at 20:23 +0800, Ayaka wrote: Sent from my iPad > On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski > wrote: > > Hi, > >> On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote: >> I forget a important thing, for the rkvdec and rk hevc decoder, it >> would >> requests cabac table, scaling list, picture parameter set and >> reference >> picture storing in one or various of DMA buffers. I am not talking >> about >> the data been parsed, the decoder would requests a raw data. >> >> For the pps and rps, it is possible to reuse the slice header, just >> let >> the decoder know the offset from the bitstream bufer, I would >> suggest to >> add three properties(with sps) for them. But I think we need a >> method to >> mark a OUTPUT side buffer for those aux data. > > I'm quite confused about the hardware implementation then. From what > you're saying, it seems that it takes the raw bitstream elements > rather > than parsed elements. Is it really a stateless implementation? > > The stateless implementation was designed with the idea that only the > raw slice data should be passed in bitstream form to the decoder. For > H.264, it seems that some decoders also need the slice header in raw > bitstream form (because they take the full slice NAL unit), see the > discussions in this thread: > media: docs-rst: Document m2m stateless video decoder interface Stateless just mean it won’t track the previous result, but I don’t think you can define what a date the hardware would need. Even you just build a dpb for the decoder, it is still stateless, but parsing less or more data from the bitstream doesn’t stop a decoder become a stateless decoder. >>> >>> Yes fair enough, the format in which the hardware decoder takes the >>> bitstream parameters does not make it stateless or stateful per-se. >>> It's just that stateless decoders should have no particular reason for >>> parsing the bitstream on their own since the hardware can be designed >>> with registers for each relevant bitstream element to configure the >>> decoding pipeline. That's how GPU-based decoder implementations are >>> implemented (VAAPI/VDPAU/NVDEC, etc). >>> >>> So the format we have agreed on so far for the stateless interface is >>> to pass parsed elements via v4l2 control structures. >>> >>> If the hardware can only work by parsing the bitstream itself, I'm not >>> sure what the best solution would be. Reconstructing the bitstream in >>> the kernel is a pretty bad option, but so is parsing in the kernel or >>> having the data both in parsed and raw forms. Do you see another >>> possibility? >> >> Is reconstructing the bitstream so bad? The v4l2 controls provide a >> generic interface to an encoded format which the driver needs to >> convert into a sequence that the hardware can understand. Typically >> this is done by populating hardware-specific structures. Can't we >> consider that in this specific instance, the hardware-specific >> structure just happens to be identical to the original bitstream >> format? > > At maximum allowed bitrate for let's say HEVC (940MB/s iirc), yes, it > would be really really bad. In GStreamer project we have discussed for > a while (but have never done anything about) adding the ability through > a bitmask to select which part of the stream need to be parsed, as > parsing itself was causing some overhead. Maybe similar thing applies, > though as per our new design, it's the fourcc that dictate the driver > behaviour, we'd need yet another fourcc for drivers that wants the full > bitstream (which seems odd if you have already parsed everything, I > think this need some clarification). Note that I am not proposing to rebuild the *entire* bitstream in-kernel. What I am saying is that if the hardware interprets some structures (like SPS/PPS) in their raw format, this raw format could be reconstructed from the structures passed by userspace at negligible cost. Such
[PATCH] staging: speakup: fix tty-operation NULL derefs
The send_xchar() and tiocmset() tty operations are optional. Add the
missing sanity checks to prevent user-space triggerable NULL-pointer
dereferences.
Fixes: 6b9ad1c742bf ("staging: speakup: add send_xchar, tiocmset and input
functionality for tty")
Cc: stable # 4.13
Cc: Okash Khawaja
Cc: Samuel Thibault
Signed-off-by: Johan Hovold
---
drivers/staging/speakup/spk_ttyio.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/speakup/spk_ttyio.c
b/drivers/staging/speakup/spk_ttyio.c
index c92bbd05516e..005de0024dd4 100644
--- a/drivers/staging/speakup/spk_ttyio.c
+++ b/drivers/staging/speakup/spk_ttyio.c
@@ -265,7 +265,8 @@ static void spk_ttyio_send_xchar(char ch)
return;
}
- speakup_tty->ops->send_xchar(speakup_tty, ch);
+ if (speakup_tty->ops->send_xchar)
+ speakup_tty->ops->send_xchar(speakup_tty, ch);
mutex_unlock(_tty_mutex);
}
@@ -277,7 +278,8 @@ static void spk_ttyio_tiocmset(unsigned int set, unsigned
int clear)
return;
}
- speakup_tty->ops->tiocmset(speakup_tty, set, clear);
+ if (speakup_tty->ops->tiocmset)
+ speakup_tty->ops->tiocmset(speakup_tty, set, clear);
mutex_unlock(_tty_mutex);
}
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: staging/intel-ipu3: Implement lock for stream on/off operations
Hi Rajmohan,
On Tue, Jan 29, 2019 at 02:27:36PM -0800, Rajmohan Mani wrote:
> Currently concurrent stream off operations on ImgU nodes are not
> synchronized, leading to use-after-free bugs (as reported by KASAN).
>
> [ 250.090724] BUG: KASAN: use-after-free in ipu3_dmamap_free+0xc5/0x116
> [ipu3_imgu]
> [ 250.090726] Read of size 8 at addr 888127b29bc0 by task yavta/18836
> [ 250.090731] Hardware name: HP Soraka/Soraka, BIOS Google_Soraka.10431.17.0
> 03/22/2018
> [ 250.090732] Call Trace:
> [ 250.090735] dump_stack+0x6a/0xb1
> [ 250.090739] print_address_description+0x8e/0x279
> [ 250.090743] ? ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu]
> [ 250.090746] kasan_report+0x260/0x28a
> [ 250.090750] ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu]
> [ 250.090754] ipu3_css_pool_cleanup+0x24/0x37 [ipu3_imgu]
> [ 250.090759] ipu3_css_pipeline_cleanup+0x61/0xb9 [ipu3_imgu]
> [ 250.090763] ipu3_css_stop_streaming+0x1f2/0x321 [ipu3_imgu]
> [ 250.090768] imgu_s_stream+0x94/0x443 [ipu3_imgu]
> [ 250.090772] ? ipu3_vb2_buf_queue+0x280/0x280 [ipu3_imgu]
> [ 250.090775] ? vb2_dma_sg_unmap_dmabuf+0x16/0x6f [videobuf2_dma_sg]
> [ 250.090778] ? vb2_buffer_in_use+0x36/0x58 [videobuf2_common]
> [ 250.090782] ipu3_vb2_stop_streaming+0xf9/0x135 [ipu3_imgu]
>
> Implemented a lock to synchronize imgu stream on / off operations and
> the modification of streaming flag (in struct imgu_device), to prevent
> these issues.
>
> Reported-by: Laurent Pinchart
> Suggested-by: Laurent Pinchart
>
> Signed-off-by: Rajmohan Mani
> ---
> drivers/staging/media/ipu3/ipu3-v4l2.c | 6 ++
> drivers/staging/media/ipu3/ipu3.c | 3 +++
> drivers/staging/media/ipu3/ipu3.h | 4
> 3 files changed, 13 insertions(+)
>
> diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c
> b/drivers/staging/media/ipu3/ipu3-v4l2.c
> index c7936032beb9..cf7e917cd0c8 100644
> --- a/drivers/staging/media/ipu3/ipu3-v4l2.c
> +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
> @@ -507,12 +507,15 @@ static int ipu3_vb2_start_streaming(struct vb2_queue
> *vq, unsigned int count)
> goto fail_stop_pipeline;
> }
>
> + mutex_lock(>streaming_lock);
> +
You appear to be using imgu_device.lock (while searching buffers to queue
to the device) as well as imgu_video_device.lock (qbuf, dqbuf) to serialise
access to imgu_video_device.buffers list. The two locks may be acquired at
the same time but each by different processes. That needs to be addressed,
but probably not in this patch.
I wonder if it'd be more simple to use imgu->lock here instead of adding a
new one.
> /* Start streaming of the whole pipeline now */
> dev_dbg(dev, "IMGU streaming is ready to start");
> r = imgu_s_stream(imgu, true);
> if (!r)
> imgu->streaming = true;
>
> + mutex_unlock(>streaming_lock);
> return 0;
>
> fail_stop_pipeline:
> @@ -543,6 +546,8 @@ static void ipu3_vb2_stop_streaming(struct vb2_queue *vq)
> dev_err(>pci_dev->dev,
> "failed to stop subdev streaming\n");
>
> + mutex_lock(>streaming_lock);
> +
> /* Was this the first node with streaming disabled? */
> if (imgu->streaming && ipu3_all_nodes_streaming(imgu, node)) {
> /* Yes, really stop streaming now */
> @@ -552,6 +557,7 @@ static void ipu3_vb2_stop_streaming(struct vb2_queue *vq)
> imgu->streaming = false;
> }
>
> + mutex_unlock(>streaming_lock);
> ipu3_return_all_buffers(imgu, node, VB2_BUF_STATE_ERROR);
> media_pipeline_stop(>vdev.entity);
> }
> diff --git a/drivers/staging/media/ipu3/ipu3.c
> b/drivers/staging/media/ipu3/ipu3.c
> index d521b3afb8b1..2daee51cd845 100644
> --- a/drivers/staging/media/ipu3/ipu3.c
> +++ b/drivers/staging/media/ipu3/ipu3.c
> @@ -635,6 +635,7 @@ static int imgu_pci_probe(struct pci_dev *pci_dev,
> return r;
>
> mutex_init(>lock);
> + mutex_init(>streaming_lock);
> atomic_set(>qbuf_barrier, 0);
> init_waitqueue_head(>buf_drain_wq);
>
> @@ -699,6 +700,7 @@ static int imgu_pci_probe(struct pci_dev *pci_dev,
> ipu3_css_set_powerdown(_dev->dev, imgu->base);
> out_mutex_destroy:
> mutex_destroy(>lock);
> + mutex_destroy(>streaming_lock);
>
> return r;
> }
> @@ -716,6 +718,7 @@ static void imgu_pci_remove(struct pci_dev *pci_dev)
> ipu3_dmamap_exit(imgu);
> ipu3_mmu_exit(imgu->mmu);
> mutex_destroy(>lock);
> + mutex_destroy(>streaming_lock);
> }
>
> static int __maybe_unused imgu_suspend(struct device *dev)
> diff --git a/drivers/staging/media/ipu3/ipu3.h
> b/drivers/staging/media/ipu3/ipu3.h
> index 04fc99f47ebb..f732315f0701 100644
> --- a/drivers/staging/media/ipu3/ipu3.h
> +++ b/drivers/staging/media/ipu3/ipu3.h
> @@ -146,6 +146,10 @@ struct imgu_device {
>* vid_buf.list and css->queue
>*/
> struct mutex lock;
> +
> + /* Lock to protect writes to
