[PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg
Each of these drivers has a copy of the same trivial helper function to
convert the pointer argument and then call the native ioctl handler.
We now have a generic implementation of that, so use it.
Signed-off-by: Arnd Bergmann
---
drivers/char/ppdev.c | 12 +-
drivers/char/tpm/tpm_vtpm_proxy.c | 12 +-
drivers/firewire/core-cdev.c | 12 +-
drivers/hid/usbhid/hiddev.c | 11 +
drivers/hwtracing/stm/core.c | 12 +-
drivers/misc/mei/main.c | 22 +
drivers/mtd/ubi/cdev.c| 36 +++-
drivers/net/tap.c | 12 +-
drivers/staging/pi433/pi433_if.c | 12 +-
drivers/usb/core/devio.c | 16 +
drivers/vfio/vfio.c | 39 +++
drivers/vhost/net.c | 12 +-
drivers/vhost/scsi.c | 12 +-
drivers/vhost/test.c | 12 +-
drivers/vhost/vsock.c | 12 +-
fs/fat/file.c | 13 +--
16 files changed, 20 insertions(+), 237 deletions(-)
diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index 1ae77b41050a..c38a62457cf0 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -674,14 +674,6 @@ static long pp_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
return ret;
}
-#ifdef CONFIG_COMPAT
-static long pp_compat_ioctl(struct file *file, unsigned int cmd,
- unsigned long arg)
-{
- return pp_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
static int pp_open(struct inode *inode, struct file *file)
{
unsigned int minor = iminor(inode);
@@ -790,9 +782,7 @@ static const struct file_operations pp_fops = {
.write = pp_write,
.poll = pp_poll,
.unlocked_ioctl = pp_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = pp_compat_ioctl,
-#endif
+ .compat_ioctl = generic_compat_ioctl_ptrarg,
.open = pp_open,
.release= pp_release,
};
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c
b/drivers/char/tpm/tpm_vtpm_proxy.c
index 87a0ce47f201..a170f5ca7416 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -678,20 +678,10 @@ static long vtpmx_fops_ioctl(struct file *f, unsigned int
ioctl,
}
}
-#ifdef CONFIG_COMPAT
-static long vtpmx_fops_compat_ioctl(struct file *f, unsigned int ioctl,
- unsigned long arg)
-{
- return vtpmx_fops_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
-}
-#endif
-
static const struct file_operations vtpmx_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = vtpmx_fops_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = vtpmx_fops_compat_ioctl,
-#endif
+ .compat_ioctl = generic_compat_ioctl_ptrarg,
.llseek = noop_llseek,
};
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index d8e185582642..2acc0c9ddf94 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1659,14 +1659,6 @@ static long fw_device_op_ioctl(struct file *file,
return dispatch_ioctl(file->private_data, cmd, (void __user *)arg);
}
-#ifdef CONFIG_COMPAT
-static long fw_device_op_compat_ioctl(struct file *file,
- unsigned int cmd, unsigned long arg)
-{
- return dispatch_ioctl(file->private_data, cmd, compat_ptr(arg));
-}
-#endif
-
static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma)
{
struct client *client = file->private_data;
@@ -1808,7 +1800,5 @@ const struct file_operations fw_device_ops = {
.mmap = fw_device_op_mmap,
.release= fw_device_op_release,
.poll = fw_device_op_poll,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = fw_device_op_compat_ioctl,
-#endif
+ .compat_ioctl = generic_compat_ioctl_ptrarg,
};
diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index 23872d08308c..73a168f97024 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -845,13 +845,6 @@ static long hiddev_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
return r;
}
-#ifdef CONFIG_COMPAT
-static long hiddev_compat_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
-{
- return hiddev_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
static const struct file_operations hiddev_fops = {
.owner =THIS_MODULE,
.read = hiddev_read,
@@ -861,9 +854,7 @@ static const struct file_operations hiddev_fops = {
.release = hiddev_release,
.unlocked_ioctl = hiddev_ioctl,
.fasync = hiddev_fasync,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = hiddev_compat_ioctl,
-#endif
+ .com
[PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
The .ioctl and .compat_ioctl file operations have the same prototype so
they can both point to the same function, which works great almost all
the time when all the commands are compatible.
One exception is the s390 architecture, where a compat pointer is only
31 bit wide, and converting it into a 64-bit pointer requires calling
compat_ptr(). Most drivers here will ever run in s390, but since we now
have a generic helper for it, it's easy enough to use it consistently.
I double-checked all these drivers to ensure that all ioctl arguments
are used as pointers or are ignored, but are not interpreted as integer
values.
Signed-off-by: Arnd Bergmann
---
drivers/android/binder.c| 2 +-
drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +-
drivers/dma-buf/dma-buf.c | 4 +---
drivers/dma-buf/sw_sync.c | 2 +-
drivers/dma-buf/sync_file.c | 2 +-
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c| 2 +-
drivers/hid/hidraw.c| 4 +---
drivers/iio/industrialio-core.c | 2 +-
drivers/infiniband/core/uverbs_main.c | 4 ++--
drivers/media/rc/lirc_dev.c | 4 +---
drivers/mfd/cros_ec_dev.c | 4 +---
drivers/misc/vmw_vmci/vmci_host.c | 2 +-
drivers/nvdimm/bus.c| 4 ++--
drivers/nvme/host/core.c| 2 +-
drivers/pci/switch/switchtec.c | 2 +-
drivers/platform/x86/wmi.c | 2 +-
drivers/rpmsg/rpmsg_char.c | 4 ++--
drivers/sbus/char/display7seg.c | 2 +-
drivers/sbus/char/envctrl.c | 4 +---
drivers/scsi/3w-.c | 4 +---
drivers/scsi/cxlflash/main.c| 2 +-
drivers/scsi/esas2r/esas2r_main.c | 2 +-
drivers/scsi/pmcraid.c | 4 +---
drivers/staging/android/ion/ion.c | 4 +---
drivers/staging/vme/devices/vme_user.c | 2 +-
drivers/tee/tee_core.c | 2 +-
drivers/usb/class/cdc-wdm.c | 2 +-
drivers/usb/class/usbtmc.c | 4 +---
drivers/video/fbdev/ps3fb.c | 2 +-
drivers/virt/fsl_hypervisor.c | 2 +-
fs/btrfs/super.c| 2 +-
fs/ceph/dir.c | 2 +-
fs/ceph/file.c | 2 +-
fs/fuse/dev.c | 2 +-
fs/notify/fanotify/fanotify_user.c | 2 +-
fs/userfaultfd.c| 2 +-
net/rfkill/core.c | 2 +-
37 files changed, 40 insertions(+), 58 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index d58763b6b009..d2464f5759f8 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5576,7 +5576,7 @@ static const struct file_operations binder_fops = {
.owner = THIS_MODULE,
.poll = binder_poll,
.unlocked_ioctl = binder_ioctl,
- .compat_ioctl = binder_ioctl,
+ .compat_ioctl = generic_compat_ioctl_ptrarg,
.mmap = binder_mmap,
.open = binder_open,
.flush = binder_flush,
diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
index abc7a7f64d64..8ff77a70addc 100644
--- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
+++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
@@ -68,7 +68,7 @@ static long adf_ctl_ioctl(struct file *fp, unsigned int cmd,
unsigned long arg);
static const struct file_operations adf_ctl_ops = {
.owner = THIS_MODULE,
.unlocked_ioctl = adf_ctl_ioctl,
- .compat_ioctl = adf_ctl_ioctl,
+ .compat_ioctl = generic_compat_ioctl_ptrarg,
};
struct adf_ctl_drv_info {
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 13884474d158..a6d7dc4cf7e9 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,9 +325,7 @@ static const struct file_operations dma_buf_fops = {
.llseek = dma_buf_llseek,
.poll = dma_buf_poll,
.unlocked_ioctl = dma_buf_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = dma_buf_ioctl,
-#endif
+ .compat_ioctl = generic_compat_ioctl_ptrarg,
};
/*
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 53c1d6d36a64..bc810506d487 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -419,5 +419,5 @@ const struct file_operations sw_sync_debugfs_fops = {
.open = sw_sync_debugfs_open,
.release= sw_sync_debugfs_release,
.unlocked_ioctl = sw_sync_ioctl,
- .compat_ioctl = sw_sync_ioctl,
+ .compat_ioctl = generic_compat_ioctl_ptrarg,
};
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 35dd06479867..1c64ed60c658 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -488,5 +488,5 @@ static const struct file_opera
Re: [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg
On Wed, Sep 12, 2018 at 5:33 PM Jason Gunthorpe wrote: > > On Wed, Sep 12, 2018 at 05:01:03PM +0200, Arnd Bergmann wrote: > > Each of these drivers has a copy of the same trivial helper function to > > convert the pointer argument and then call the native ioctl handler. > > > > We now have a generic implementation of that, so use it. > > > > For vtpm: > > Reviewed-by: Jason Gunthorpe > > Arnd, would you consider including a patch as part of/after this > series to make compat_ioctl in drivers/infiniband/core/uverbs_main.c > use this as well? Looks like a bug too? That should be included in patch 5 in this series. I may have skipped some Cc there since it had too many recipients (sent only to the mailing lists instead). Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support
On Fri, Sep 14, 2018 at 2:52 PM Thomas Gleixner wrote: > > Matt attempted to add CLOCK_TAI support to the VDSO clock_gettime() > implementation, which extended the clockid switch case and added yet > another slightly different copy of the same code. > > Especially the extended switch case is problematic as the compiler tends to > generate a jump table which then requires to use retpolines. If jump tables > are disabled it adds yet another conditional to the existing maze. > > This series takes a different approach by consolidating the almost > identical functions into one implementation for high resolution clocks and > one for the coarse grained clock ids by storing the base data for each > clock id in an array which is indexed by the clock id. > > This completely eliminates the switch case and allows further > simplifications of the code base, which at the end all together gain a few > cycles performance or at least stay on par with todays code. The resulting > performance depends heavily on the micro architecture and the compiler. No objections from my side, just a few general remarks: Deepa and I have discussed the vdso in the past for 2038. I have started a patch that I'll have to redo on top of yours, but that is fine, your cleanup is only going to help here. More generally speaking, Deepa said that she would like to see some generalization on the vdso before adding the time64_t based variants. Your series probably makes x86 diverge more from the others, which makes it harder to consolidate them again, but we might just as well use your new implementation to base the generic one on, and just move the other ones over to that. A couple of architectures (s390, ia64, riscv, powerpc, arm64) implement the vdso as assembler code at the moment, so they won't be as easy to consolidate (other than outright replacing all the code). The other five: arch/x86/entry/vdso/vclock_gettime.c arch/sparc/vdso/vclock_gettime.c arch/nds32/kernel/vdso/gettimeofday.c arch/mips/vdso/gettimeofday.c arch/arm/vdso/vgettimeofday.c are basically all minor variations of the same code base and could be consolidated to some degree. Any suggestions here? Should we plan to do that consolitdation based on your new version, or just add clock_gettime64 in arm32 and x86-32, and then be done with it? The other ones will obviously still be fast for 32-bit time_t and will have a working non-vdso sys_clock_getttime64(). I also wonder about clock_getres(): half the architectures seem to implement it in vdso, but notably arm32 and x86 don't, and I had not expected it to be performance critical given that the result is easily cached in user space. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe wrote: > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > > > Acked-by: Darren Hart (VMware) > > > > > > > > As for a longer term solution, would it be possible to init fops in such > > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > > structure? > > > > > > Bad idea, that... Because several years down the road somebody will > > > add > > > an ioctl that takes an unsigned int for argument. Without so much as > > > looking > > > at your magical mystery macro being used to initialize file_operations. > > > > Fair, being explicit in the declaration as it is currently may be > > preferable then. > > It would be much cleaner and safer if you could arrange things to add > something like this to struct file_operations: > > long (*ptr_ioctl) (struct file *, unsigned int, void __user *); > > Where the core code automatically converts the unsigned long to the > void __user * as appropriate. > > Then it just works right always and the compiler will help address > Al's concern down the road. I think if we wanted to do this with a new file operation, the best way would be to do the copy_from_user()/copy_to_user() in the caller as well. We already do this inside of some subsystems, notably drivers/media/, and it simplifies the implementation of the ioctl handler function significantly. We obviously cannot do this in general, both because of traditional drivers that have 16-bit command codes (drivers/tty and others) and also because of drivers that by accident defined the commands incorrectly and use the wrong type or the wrong direction in the definition. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 00/11] x86/vdso: Cleanups, simmplifications and CLOCK_TAI support
On Mon, Sep 17, 2018 at 3:00 PM Thomas Gleixner wrote: > > On Fri, 14 Sep 2018, Arnd Bergmann wrote: > > On Fri, Sep 14, 2018 at 2:52 PM Thomas Gleixner wrote: > > A couple of architectures (s390, ia64, riscv, powerpc, arm64) > > implement the vdso as assembler code at the moment, so they > > won't be as easy to consolidate (other than outright replacing all > > the code). > > > > The other five: > > arch/x86/entry/vdso/vclock_gettime.c > > arch/sparc/vdso/vclock_gettime.c > > arch/nds32/kernel/vdso/gettimeofday.c > > arch/mips/vdso/gettimeofday.c > > arch/arm/vdso/vgettimeofday.c > > > > are basically all minor variations of the same code base and could be > > consolidated to some degree. > > Any suggestions here? Should we plan to do that consolitdation based on > > your new version, or just add clock_gettime64 in arm32 and x86-32, and then > > be done with it? The other ones will obviously still be fast for 32-bit > > time_t > > and will have a working non-vdso sys_clock_getttime64(). > > In principle consolidating all those implementations should be possible to > some extent and probably worthwhile. What's arch specific are the actual > accessors to the hardware clocks. Ok. > > I also wonder about clock_getres(): half the architectures seem to implement > > it in vdso, but notably arm32 and x86 don't, and I had not expected it to be > > performance critical given that the result is easily cached in user space. > > getres() is not really performance critical, but adding it does not create > a huge problem either. Right, I'd just not add a getres_time64() to the vdso then. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
On Mon, Sep 24, 2018 at 10:35 PM Jason Gunthorpe wrote: > On Mon, Sep 24, 2018 at 10:18:52PM +0200, Arnd Bergmann wrote: > > On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe wrote: > > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > We already do this inside of some subsystems, notably drivers/media/, > > and it simplifies the implementation of the ioctl handler function > > significantly. We obviously cannot do this in general, both because of > > traditional drivers that have 16-bit command codes (drivers/tty and others) > > and also because of drivers that by accident defined the commands > > incorrectly and use the wrong type or the wrong direction in the > > definition. > > That could work well, but the first idea could be done globally and > mechanically, while this would require very careful per-driver > investigation. > > Particularly if the core code has worse performance.. ie due to > kmalloc calls or something. > > I think it would make more sense to start by having the core do the > case to __user and then add another entry point to have the core do > the copy_from_user, and so on. Having six separate callback pointers to implement a single system call seems a bit excessive though. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vchiq: rework remove_event handling
I had started the removal of semaphores in this driver without knowing
that Nicolas Saenz Julienne also worked on this. In case of the "remote
event" infrastructure, my solution seemed significantly better, so I'm
proposing this as a change on top.
The problem with using either semaphores or completions here is that
it's an overly complex way of waking up a thread, and it looks like the
'count' of the semaphore can easily get out of sync, even though I found
it hard to come up with a specific example.
Changing it to a 'wait_queue_head_t' instead of a completion simplifies
this by letting us wait directly on the 'event->fired' variable that is
set by the videocore.
Another simplification is passing the wait queue directly into the helper
functions instead of going through the fragile logic of recording the
offset inside of a structure as part of a shared memory variable. This
also avoids one uncached memory read and should be faster.
Note that I'm changing it back to 'killable' after the previous patch
changed 'killable' to 'interruptible', apparently based on a misunderstanding
of the subtle down_interruptible() macro override in vchiq_killable.h.
Fixes: f27e47bc6b8b ("staging: vchiq: use completions instead of semaphores")
Signed-off-by: Arnd Bergmann
---
.../interface/vchiq_arm/vchiq_core.c | 63 +++
.../interface/vchiq_arm/vchiq_core.h | 12 ++--
2 files changed, 30 insertions(+), 45 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 482b5daf6c0c..eda3004a0c6a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -417,26 +417,23 @@ vchiq_set_conn_state(VCHIQ_STATE_T *state,
VCHIQ_CONNSTATE_T newstate)
}
static inline void
-remote_event_create(REMOTE_EVENT_T *event)
+remote_event_create(wait_queue_head_t *wq, REMOTE_EVENT_T *event)
{
event->armed = 0;
/* Don't clear the 'fired' flag because it may already have been set
** by the other side. */
+ init_waitqueue_head(wq);
}
static inline int
-remote_event_wait(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event)
+remote_event_wait(wait_queue_head_t *wq, REMOTE_EVENT_T *event)
{
if (!event->fired) {
event->armed = 1;
dsb(sy);
- if (!event->fired) {
- if (wait_for_completion_interruptible(
- (struct completion *)
- ((char *)state + event->event))) {
- event->armed = 0;
- return 0;
- }
+ if (wait_event_killable(*wq, event->fired)) {
+ event->armed = 0;
+ return 0;
}
event->armed = 0;
wmb();
@@ -447,26 +444,26 @@ remote_event_wait(VCHIQ_STATE_T *state, REMOTE_EVENT_T
*event)
}
static inline void
-remote_event_signal_local(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event)
+remote_event_signal_local(wait_queue_head_t *wq, REMOTE_EVENT_T *event)
{
event->armed = 0;
- complete((struct completion *)((char *)state + event->event));
+ wake_up_all(wq);
}
static inline void
-remote_event_poll(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event)
+remote_event_poll(wait_queue_head_t *wq, REMOTE_EVENT_T *event)
{
if (event->fired && event->armed)
- remote_event_signal_local(state, event);
+ remote_event_signal_local(wq, event);
}
void
remote_event_pollall(VCHIQ_STATE_T *state)
{
- remote_event_poll(state, &state->local->sync_trigger);
- remote_event_poll(state, &state->local->sync_release);
- remote_event_poll(state, &state->local->trigger);
- remote_event_poll(state, &state->local->recycle);
+ remote_event_poll(&state->sync_trigger_event,
&state->local->sync_trigger);
+ remote_event_poll(&state->sync_release_event,
&state->local->sync_release);
+ remote_event_poll(&state->trigger_event, &state->local->trigger);
+ remote_event_poll(&state->recycle_event, &state->local->recycle);
}
/* Round up message sizes so that any space at the end of a slot is always big
@@ -550,7 +547,7 @@ request_poll(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T
*service, int poll_type)
wmb();
/* ... and ensure the slot handler runs. */
- remote_event_signal_local(state, &state->local->trigger);
+ remote_event_signal_local(&state->trigger_event,
&stat
[PATCH 3/3] staging: rtl8723bs: remove semaphore remnants
Nothing uses the semaphores any more in this driver, so remove
all references to that type.
Signed-off-by: Arnd Bergmann
---
drivers/staging/rtl8723bs/core/rtw_pwrctrl.c| 1 -
drivers/staging/rtl8723bs/include/osdep_service_linux.h | 2 --
drivers/staging/rtl8723bs/include/rtw_io.h | 1 -
drivers/staging/rtl8723bs/include/rtw_mp.h | 1 -
drivers/staging/rtl8723bs/include/rtw_pwrctrl.h | 5 -
5 files changed, 10 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
index b7dd5fec9b31..eb27522a5444 100644
--- a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
+++ b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
@@ -1126,7 +1126,6 @@ void rtw_init_pwrctrl_priv(struct adapter *padapter)
struct pwrctrl_priv *pwrctrlpriv = adapter_to_pwrctl(padapter);
mutex_init(&pwrctrlpriv->lock);
- sema_init(&pwrctrlpriv->check_32k_lock, 1);
pwrctrlpriv->rf_pwrstate = rf_on;
pwrctrlpriv->ips_enter_cnts = 0;
pwrctrlpriv->ips_leave_cnts = 0;
diff --git a/drivers/staging/rtl8723bs/include/osdep_service_linux.h
b/drivers/staging/rtl8723bs/include/osdep_service_linux.h
index 58d1e1019241..2f1b51e614fb 100644
--- a/drivers/staging/rtl8723bs/include/osdep_service_linux.h
+++ b/drivers/staging/rtl8723bs/include/osdep_service_linux.h
@@ -22,7 +22,6 @@
#include
#include
#include
- #include
#include
#include
#include
@@ -41,7 +40,6 @@
#include
#include
- typedef struct semaphore _sema;
typedef spinlock_t _lock;
typedef struct mutex_mutex;
typedef struct timer_list _timer;
diff --git a/drivers/staging/rtl8723bs/include/rtw_io.h
b/drivers/staging/rtl8723bs/include/rtw_io.h
index 4f8be55da65d..99d104b3647a 100644
--- a/drivers/staging/rtl8723bs/include/rtw_io.h
+++ b/drivers/staging/rtl8723bs/include/rtw_io.h
@@ -115,7 +115,6 @@ struct io_req {
u32 command;
u32 status;
u8 *pbuf;
- _sema sema;
void (*_async_io_callback)(struct adapter *padater, struct io_req
*pio_req, u8 *cnxt);
u8 *cnxt;
diff --git a/drivers/staging/rtl8723bs/include/rtw_mp.h
b/drivers/staging/rtl8723bs/include/rtw_mp.h
index 839084733201..bb3970d58573 100644
--- a/drivers/staging/rtl8723bs/include/rtw_mp.h
+++ b/drivers/staging/rtl8723bs/include/rtw_mp.h
@@ -62,7 +62,6 @@ typedef struct _MPT_CONTEXT
/* Indicate if the driver is unloading or unloaded. */
boolbMptDrvUnload;
- _sema MPh2c_Sema;
_timer MPh2c_timeout_timer;
/* Event used to sync H2c for BT control */
diff --git a/drivers/staging/rtl8723bs/include/rtw_pwrctrl.h
b/drivers/staging/rtl8723bs/include/rtw_pwrctrl.h
index abf48ae01900..e2a4c680125f 100644
--- a/drivers/staging/rtl8723bs/include/rtw_pwrctrl.h
+++ b/drivers/staging/rtl8723bs/include/rtw_pwrctrl.h
@@ -94,10 +94,6 @@ struct reportpwrstate_parm {
unsigned short rsvd;
};
-
-typedef _sema _pwrlock;
-
-
#define LPS_DELAY_TIME 1*HZ /* 1 sec */
#define EXE_PWR_NONE 0x01
@@ -209,7 +205,6 @@ typedef struct pno_scan_info
struct pwrctrl_priv
{
struct mutex lock;
- _pwrlockcheck_32k_lock;
volatile u8 rpwm; /* requested power state for fw */
volatile u8 cpwm; /* fw current power state. updated when 1. read from
HCPWM 2. driver lowers power level */
volatile u8 tog; /* toggling */
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: rtl8723bs: change semaphores to completions
This driver uses many semaphores, most of them are equivalent to
completions. The other copies of this driver got moved over to
completions a while ago, so do the same here.
In this usage scenario, the two are equivalent, so the behavior
should not change.
Signed-off-by: Arnd Bergmann
---
drivers/staging/rtl8723bs/core/rtw_cmd.c | 20 -
drivers/staging/rtl8723bs/core/rtw_mlme.c | 2 --
drivers/staging/rtl8723bs/core/rtw_pwrctrl.c | 4 ++--
drivers/staging/rtl8723bs/core/rtw_xmit.c | 8 +++
.../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 4 ++--
.../staging/rtl8723bs/hal/rtl8723bs_xmit.c| 22 ---
drivers/staging/rtl8723bs/hal/sdio_ops.c | 2 +-
drivers/staging/rtl8723bs/include/rtw_cmd.h | 7 +++---
drivers/staging/rtl8723bs/include/rtw_xmit.h | 9
drivers/staging/rtl8723bs/os_dep/os_intfs.c | 6 ++---
drivers/staging/rtl8723bs/os_dep/xmit_linux.c | 2 +-
11 files changed, 39 insertions(+), 47 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c
b/drivers/staging/rtl8723bs/core/rtw_cmd.c
index 830be63391b7..3d206c33eafa 100644
--- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
@@ -166,10 +166,8 @@ sint _rtw_init_cmd_priv(struct cmd_priv
*pcmdpriv)
{
sint res = _SUCCESS;
- sema_init(&(pcmdpriv->cmd_queue_sema), 0);
- /* sema_init(&(pcmdpriv->cmd_done_sema), 0); */
- sema_init(&(pcmdpriv->terminate_cmdthread_sema), 0);
-
+ init_completion(&pcmdpriv->cmd_queue_comp);
+ init_completion(&pcmdpriv->terminate_cmdthread_comp);
_rtw_init_queue(&(pcmdpriv->cmd_queue));
@@ -369,7 +367,7 @@ u32 rtw_enqueue_cmd(struct cmd_priv *pcmdpriv, struct
cmd_obj *cmd_obj)
res = _rtw_enqueue_cmd(&pcmdpriv->cmd_queue, cmd_obj);
if (res == _SUCCESS)
- up(&pcmdpriv->cmd_queue_sema);
+ complete(&pcmdpriv->cmd_queue_comp);
exit:
return res;
@@ -410,8 +408,8 @@ void rtw_stop_cmd_thread(struct adapter *adapter)
atomic_read(&(adapter->cmdpriv.cmdthd_running)) == true &&
adapter->cmdpriv.stop_req == 0) {
adapter->cmdpriv.stop_req = 1;
- up(&adapter->cmdpriv.cmd_queue_sema);
- down(&adapter->cmdpriv.terminate_cmdthread_sema);
+ complete(&adapter->cmdpriv.cmd_queue_comp);
+ wait_for_completion(&adapter->cmdpriv.terminate_cmdthread_comp);
}
}
@@ -435,13 +433,13 @@ int rtw_cmd_thread(void *context)
pcmdpriv->stop_req = 0;
atomic_set(&(pcmdpriv->cmdthd_running), true);
- up(&pcmdpriv->terminate_cmdthread_sema);
+ complete(&pcmdpriv->terminate_cmdthread_comp);
RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_, ("start r871x
rtw_cmd_thread \n"));
while (1) {
- if (down_interruptible(&pcmdpriv->cmd_queue_sema)) {
- DBG_871X_LEVEL(_drv_always_, FUNC_ADPT_FMT"
down_interruptible(&pcmdpriv->cmd_queue_sema) return != 0, break\n",
FUNC_ADPT_ARG(padapter));
+ if
(wait_for_completion_interruptible(&pcmdpriv->cmd_queue_comp)) {
+ DBG_871X_LEVEL(_drv_always_, FUNC_ADPT_FMT"
wait_for_completion_interruptible(&pcmdpriv->cmd_queue_comp) return != 0,
break\n", FUNC_ADPT_ARG(padapter));
break;
}
@@ -581,7 +579,7 @@ int rtw_cmd_thread(void *context)
rtw_free_cmd_obj(pcmd);
} while (1);
- up(&pcmdpriv->terminate_cmdthread_sema);
+ complete(&pcmdpriv->terminate_cmdthread_comp);
atomic_set(&(pcmdpriv->cmdthd_running), false);
thread_exit();
diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c
b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index 4c5d5cf9dfe0..81505f3bce8b 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -2373,8 +2373,6 @@ sint rtw_set_key(struct adapter *adapter, struct
security_priv *psecuritypriv, s
INIT_LIST_HEAD(&pcmd->list);
- /* sema_init(&(pcmd->cmd_sem), 0); */
-
res = rtw_enqueue_cmd(pcmdpriv, pcmd);
} else{
setkey_hdl(adapter, (u8 *)psetkeyparm);
diff --git a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
index 59a667753266..a0d8fbeecf9b 100644
--- a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
+++ b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
@@ -745,10 +745,10 @@ void cpwm_int_hdl(
if (pwrpriv->cpwm >= PS_STATE_S2) {
if (pwrpriv->alives & CMD_ALIVE)
- up(&padapter-&g
[PATCH 2/3] staging: rtl8723bs: change pwrctrl lock to a mutex
This semaphore is used like a mutex, so it should use the regular
mutex API, as we do in the other copies of this driver.
Signed-off-by: Arnd Bergmann
---
drivers/staging/rtl8723bs/core/rtw_cmd.c | 4 +-
drivers/staging/rtl8723bs/core/rtw_pwrctrl.c | 73 +--
.../staging/rtl8723bs/include/rtw_pwrctrl.h | 3 +-
3 files changed, 40 insertions(+), 40 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c
b/drivers/staging/rtl8723bs/core/rtw_cmd.c
index 3d206c33eafa..984db6f2217c 100644
--- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
@@ -1621,7 +1621,7 @@ static void rtw_lps_change_dtim_hdl(struct adapter
*padapter, u8 dtim)
if (rtw_btcoex_IsBtControlLps(padapter) == true)
return;
- down(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->lock);
if (pwrpriv->dtim != dtim) {
DBG_871X("change DTIM from %d to %d, bFwCurrentInPSMode =%d,
ps_mode =%d\n", pwrpriv->dtim, dtim,
@@ -1638,7 +1638,7 @@ static void rtw_lps_change_dtim_hdl(struct adapter
*padapter, u8 dtim)
rtw_hal_set_hwreg(padapter, HW_VAR_H2C_FW_PWRMODE, (u8
*)(&ps_mode));
}
- up(&pwrpriv->lock);
+ mutex_unlock(&pwrpriv->lock);
}
static void rtw_dm_ra_mask_hdl(struct adapter *padapter, struct sta_info *psta)
diff --git a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
index a0d8fbeecf9b..b7dd5fec9b31 100644
--- a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
+++ b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c
@@ -45,9 +45,9 @@ void ips_enter(struct adapter *padapter)
rtw_btcoex_IpsNotify(padapter, pwrpriv->ips_mode_req);
- down(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->lock);
_ips_enter(padapter);
- up(&pwrpriv->lock);
+ mutex_unlock(&pwrpriv->lock);
}
int _ips_leave(struct adapter *padapter)
@@ -85,9 +85,9 @@ int ips_leave(struct adapter *padapter)
if (!is_primary_adapter(padapter))
return _SUCCESS;
- down(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->lock);
ret = _ips_leave(padapter);
- up(&pwrpriv->lock);
+ mutex_unlock(&pwrpriv->lock);
if (_SUCCESS == ret)
rtw_btcoex_IpsNotify(padapter, IPS_NONE);
@@ -158,9 +158,9 @@ void rtw_ps_processor(struct adapter *padapter)
struct debug_priv *pdbgpriv = &psdpriv->drv_dbg;
u32 ps_deny = 0;
- down(&adapter_to_pwrctl(padapter)->lock);
+ mutex_lock(&adapter_to_pwrctl(padapter)->lock);
ps_deny = rtw_ps_deny_get(padapter);
- up(&adapter_to_pwrctl(padapter)->lock);
+ mutex_unlock(&adapter_to_pwrctl(padapter)->lock);
if (ps_deny != 0) {
DBG_871X(FUNC_ADPT_FMT ": ps_deny = 0x%08X, skip power save!\n",
FUNC_ADPT_ARG(padapter), ps_deny);
@@ -413,7 +413,7 @@ void rtw_set_ps_mode(struct adapter *padapter, u8 ps_mode,
u8 smart_ps, u8 bcn_a
return;
- down(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->lock);
/* if (pwrpriv->pwr_mode == PS_MODE_ACTIVE) */
if (ps_mode == PS_MODE_ACTIVE) {
@@ -494,7 +494,7 @@ void rtw_set_ps_mode(struct adapter *padapter, u8 ps_mode,
u8 smart_ps, u8 bcn_a
}
}
- up(&pwrpriv->lock);
+ mutex_unlock(&pwrpriv->lock);
}
/*
@@ -628,11 +628,11 @@ void LeaveAllPowerSaveModeDirect(struct adapter *Adapter)
return;
}
- down(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->lock);
rtw_set_rpwm(Adapter, PS_STATE_S4);
- up(&pwrpriv->lock);
+ mutex_unlock(&pwrpriv->lock);
rtw_lps_ctrl_wk_cmd(pri_padapter, LPS_CTRL_LEAVE, 0);
} else{
@@ -696,7 +696,7 @@ void LPS_Leave_check(
cond_resched();
while (1) {
- down(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->lock);
if ((padapter->bSurpriseRemoved == true)
|| (padapter->hw_init_completed == false)
@@ -704,7 +704,7 @@ void LPS_Leave_check(
)
bReady = true;
- up(&pwrpriv->lock);
+ mutex_unlock(&pwrpriv->lock);
if (true == bReady)
break;
@@ -732,11 +732,10 @@ void cpwm_int_hdl(
pwrpriv = adapter_to_pwrctl(padapter);
- down(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->lock);
if (pwrpriv->rpwm < PS_STATE_S2) {
DBG_871X("%s: Redundant CPWM Int. RPWM = 0x%02X CPWM =
0x%02x\n", __func__, pwrp
[PATCH] staging: speakup: change semaphore to completion
In this driver, both function the same way, but we want to eventually
kill off semaphores, so a completion is the better choice here.
Signed-off-by: Arnd Bergmann
---
drivers/staging/speakup/spk_ttyio.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/speakup/spk_ttyio.c
b/drivers/staging/speakup/spk_ttyio.c
index 979e3ae249c1..c92bbd05516e 100644
--- a/drivers/staging/speakup/spk_ttyio.c
+++ b/drivers/staging/speakup/spk_ttyio.c
@@ -10,7 +10,7 @@
struct spk_ldisc_data {
char buf;
- struct semaphore sem;
+ struct completion completion;
bool buf_free;
};
@@ -55,7 +55,7 @@ static int spk_ttyio_ldisc_open(struct tty_struct *tty)
if (!ldisc_data)
return -ENOMEM;
- sema_init(&ldisc_data->sem, 0);
+ init_completion(&ldisc_data->completion);
ldisc_data->buf_free = true;
speakup_tty->disc_data = ldisc_data;
@@ -95,7 +95,7 @@ static int spk_ttyio_receive_buf2(struct tty_struct *tty,
ldisc_data->buf = cp[0];
ldisc_data->buf_free = false;
- up(&ldisc_data->sem);
+ complete(&ldisc_data->completion);
return 1;
}
@@ -286,7 +286,8 @@ static unsigned char ttyio_in(int timeout)
struct spk_ldisc_data *ldisc_data = speakup_tty->disc_data;
char rv;
- if (down_timeout(&ldisc_data->sem, usecs_to_jiffies(timeout)) ==
-ETIME) {
+ if (wait_for_completion_timeout(&ldisc_data->completion,
+ usecs_to_jiffies(timeout)) == 0) {
if (timeout)
pr_warn("spk_ttyio: timeout (%d) while waiting for
input\n",
timeout);
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vchiq: rework remove_event handling
On Tue, Dec 11, 2018 at 1:31 PM Nicolas Saenz Julienne wrote: > > On Tue, 2018-12-11 at 13:07 +0300, Dan Carpenter wrote: > > On Mon, Dec 10, 2018 at 10:11:58PM +0100, Arnd Bergmann wrote: > > > Note that I'm changing it back to 'killable' after the previous > > > patch > > > changed 'killable' to 'interruptible', apparently based on a > > > misunderstanding > > > of the subtle down_interruptible() macro override in > > > vchiq_killable.h. > > > > Oh wow... The non standard down_interruptible() macro is ugly. > > > > Same reaction here, *_*. I wasn't even aware of it. > > Arnd, as long as it you're not doing it already. I'll prepare some > extra fixes for the rest of completions and get rid of that file > altogether. Ok, please do that, and add me to Cc. I think there is one counting semaphore left (g_free_fragments_sema), which could just use the plain down_killable() before you remove the wrapper. I actually have another (longer) patch series that is not entirely ready to convert all counting semaphores into a new 'struct counting_semaphore' to split them away from the semaphores that can be converted into mutexes, completions or other replacements. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vchiq: rework remove_event handling
On Tue, Dec 11, 2018 at 1:36 PM Nicolas Saenz Julienne
wrote:
> On Mon, 2018-12-10 at 22:11 +0100, Arnd Bergmann wrote:
> > @@ -447,26 +444,26 @@ remote_event_wait(VCHIQ_STATE_T *state,
> > REMOTE_EVENT_T *event)
> > }
> >
> > static inline void
> > -remote_event_signal_local(VCHIQ_STATE_T *state, REMOTE_EVENT_T
> > *event)
> > +remote_event_signal_local(wait_queue_head_t *wq, REMOTE_EVENT_T
> > *event)
> > {
> > event->armed = 0;
> > - complete((struct completion *)((char *)state + event->event));
> > + wake_up_all(wq);
>
> Shouldn't this just be "wake_up(wq)"?
I wasn't entirely sure if we could get with more than one thread waiting
for the wakeup. With the semaphore or completion that would already
be broken because we'd only wake up one of them, but I was hoping
to stay on the safe side with wake_up_all().
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt665x: fix alignment constraints
From: Arnd Bergmann
multiple structures contains a ieee80211_rts structure, which is required to
have at least two byte alignment, but are annotated with a __packed attribute
to force single-byte alignment:
staging/vt6656/rxtx.h:98:1: warning: alignment 1 of 'struct vnt_rts_g' is less
than 2 [-Wpacked-not-aligned]
staging/vt6656/rxtx.h:106:1: warning: alignment 1 of 'struct vnt_rts_ab' is
less than 2 [-Wpacked-not-aligned]
staging/vt6656/rxtx.h:116:1: warning: alignment 1 of 'struct vnt_cts' is less
than 2 [-Wpacked-not-aligned]
I see no reason why the structure itself would be misaligned, and all members
have at least two-byte alignment within the structure, so use the same
constraint on the sturcture itself.
Signed-off-by: Arnd Bergmann
---
drivers/staging/vt6655/rxtx.h | 8
drivers/staging/vt6656/rxtx.h | 6 +++---
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/vt6655/rxtx.h b/drivers/staging/vt6655/rxtx.h
index 464dd89078b2..e7061d383306 100644
--- a/drivers/staging/vt6655/rxtx.h
+++ b/drivers/staging/vt6655/rxtx.h
@@ -111,7 +111,7 @@ struct vnt_rts_g {
__le16 duration_bb;
u16 reserved;
struct ieee80211_rts data;
-} __packed;
+} __packed __aligned(2);
struct vnt_rts_g_fb {
struct vnt_phy_field b;
@@ -125,14 +125,14 @@ struct vnt_rts_g_fb {
__le16 rts_duration_ba_f1;
__le16 rts_duration_aa_f1;
struct ieee80211_rts data;
-} __packed;
+} __packed __aligned(2);
struct vnt_rts_ab {
struct vnt_phy_field ab;
__le16 duration;
u16 reserved;
struct ieee80211_rts data;
-} __packed;
+} __packed __aligned(2);
struct vnt_rts_a_fb {
struct vnt_phy_field a;
@@ -141,7 +141,7 @@ struct vnt_rts_a_fb {
__le16 rts_duration_f0;
__le16 rts_duration_f1;
struct ieee80211_rts data;
-} __packed;
+} __packed __aligned(2);
/* CTS buffer header */
struct vnt_cts {
diff --git a/drivers/staging/vt6656/rxtx.h b/drivers/staging/vt6656/rxtx.h
index 6ca2ca32d036..f23440799443 100644
--- a/drivers/staging/vt6656/rxtx.h
+++ b/drivers/staging/vt6656/rxtx.h
@@ -95,7 +95,7 @@ struct vnt_rts_g {
u16 wReserved;
struct ieee80211_rts data;
struct vnt_tx_datahead_g data_head;
-} __packed;
+} __packed __aligned(2);
struct vnt_rts_ab {
struct vnt_phy_field ab;
@@ -103,7 +103,7 @@ struct vnt_rts_ab {
u16 wReserved;
struct ieee80211_rts data;
struct vnt_tx_datahead_ab data_head;
-} __packed;
+} __packed __aligned(2);
/* CTS buffer header */
struct vnt_cts {
@@ -113,7 +113,7 @@ struct vnt_cts {
struct ieee80211_cts data;
u16 reserved2;
struct vnt_tx_datahead_g data_head;
-} __packed;
+} __packed __aligned(2);
union vnt_tx_data_head {
/* rts g */
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723bs: remove unused structures
From: Arnd Bergmann
Building this with 'make W=1' produces a couple of warnings:
rtl8723bs/include/ieee80211.h:730:1: warning: alignment 1 of 'struct
ieee80211_assoc_request_frame' is less than 2 [-Wpacked-not-aligned]
rtl8723bs/include/ieee80211.h:737:1: warning: alignment 1 of 'struct
ieee80211_assoc_response_frame' is less than 2 [-Wpacked-not-aligned]
The warnings are in dead code, so just remove the bits that
are obviously broken like this.
Signed-off-by: Arnd Bergmann
---
drivers/staging/rtl8723bs/include/ieee80211.h | 79 ---
1 file changed, 79 deletions(-)
diff --git a/drivers/staging/rtl8723bs/include/ieee80211.h
b/drivers/staging/rtl8723bs/include/ieee80211.h
index d9ff8c8e7f36..f80db2c984a4 100644
--- a/drivers/staging/rtl8723bs/include/ieee80211.h
+++ b/drivers/staging/rtl8723bs/include/ieee80211.h
@@ -667,85 +667,6 @@ struct ieee80211_header_data {
#define MFIE_TYPE_RATES_EX 50
#define MFIE_TYPE_GENERIC221
-struct ieee80211_info_element_hdr {
- u8 id;
- u8 len;
-} __attribute__ ((packed));
-
-struct ieee80211_info_element {
- u8 id;
- u8 len;
- u8 data[0];
-} __attribute__ ((packed));
-
-/*
- * These are the data types that can make up management packets
- *
- u16 auth_algorithm;
- u16 auth_sequence;
- u16 beacon_interval;
- u16 capability;
- u8 current_ap[ETH_ALEN];
- u16 listen_interval;
- struct {
- u16 association_id:14, reserved:2;
- } __attribute__ ((packed));
- u32 time_stamp[2];
- u16 reason;
- u16 status;
-*/
-
-#define IEEE80211_DEFAULT_TX_ESSID "Penguin"
-#define IEEE80211_DEFAULT_BASIC_RATE 10
-
-
-struct ieee80211_authentication {
- struct ieee80211_header_data header;
- u16 algorithm;
- u16 transaction;
- u16 status;
- /* struct ieee80211_info_element_hdr info_element; */
-} __attribute__ ((packed));
-
-
-struct ieee80211_probe_response {
- struct ieee80211_header_data header;
- u32 time_stamp[2];
- u16 beacon_interval;
- u16 capability;
- struct ieee80211_info_element info_element;
-} __attribute__ ((packed));
-
-struct ieee80211_probe_request {
- struct ieee80211_header_data header;
- /*struct ieee80211_info_element info_element;*/
-} __attribute__ ((packed));
-
-struct ieee80211_assoc_request_frame {
- struct ieee80211_hdr_3addr header;
- u16 capability;
- u16 listen_interval;
- /* u8 current_ap[ETH_ALEN]; */
- struct ieee80211_info_element_hdr info_element;
-} __attribute__ ((packed));
-
-struct ieee80211_assoc_response_frame {
- struct ieee80211_hdr_3addr header;
- u16 capability;
- u16 status;
- u16 aid;
-} __attribute__ ((packed));
-
-struct ieee80211_txb {
- u8 nr_frags;
- u8 encrypted;
- u16 reserved;
- u16 frag_size;
- u16 payload_size;
- struct sk_buff *fragments[0];
-};
-
-
/* SWEEP TABLE ENTRIES NUMBER*/
#define MAX_SWEEP_TAB_ENTRIES42
#define MAX_SWEEP_TAB_ENTRIES_PER_PACKET 7
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vc04_services: Fixed address type mismatch in vchiq_arm.c
On Thu, Feb 18, 2021 at 10:39 AM Greg KH wrote: > > On Thu, Feb 18, 2021 at 02:40:15PM +0530, Pritthijit Nath wrote: > > This change fixes a sparse address type mismatch warning "incorrect type > > in assignment (different address spaces)". > > > > Signed-off-by: Pritthijit Nath > > --- > > .../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 6 +- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > > index 59e45dc03a97..3c715b926a57 100644 > > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > > @@ -1214,11 +1214,7 @@ static int vchiq_ioc_await_completion(struct > > vchiq_instance *instance, > > !instance->use_close_delivered) > > unlock_service(service); > > > > - /* > > - * FIXME: address space mismatch, does bulk_userdata > > - * actually point to user or kernel memory? > > - */ > > - user_completion.bulk_userdata = completion->bulk_userdata; > > + user_completion.bulk_userdata = (void __user > > *)completion->bulk_userdata; > > So, this pointer really is user memory? > > How did you determine that? > > If so, why isn't this a __user * in the first place? > > You can't just paper over the FIXME by doing a cast without doing the > real work here, otherwise someone wouldn't have written the FIXME :) Agreed. I added the FIXME as part of a cleanup work I did last year. The obvious step is to mark the corresponding field in vchiq_completion_data_kernel as a __user pointer, and then check all assignments *to* that members to ensure they all refer to __user pointers as well. At some point I gave up here, as far as I recall there were certain assignments that were clearly kernel data, in particular the vchiq_service_params_kernel->callback() argument seems to sometimes come from kmalloc() and must not be passed down to user space. The alternative would be to look at the user space side to figure out how the returned data is actually used. If user space doesn't rely on it, it can simply get set to NULL, and if it does use it, then the question is which code path in the kernel correctly assigns it. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wimax/i2400m: don't change the endianness of one byte variable
On Thu, Feb 18, 2021 at 10:54 AM Muhammad Usama Anjum wrote: > > On Thu, 2021-02-18 at 10:40 +0100, Greg KH wrote: > > On Thu, Feb 18, 2021 at 02:21:54PM +0500, Muhammad Usama Anjum wrote: > > > It is wrong to change the endianness of a variable which has just one > > > byte size. > > > > > > Sparse warnings fixed: > > > drivers/staging//wimax/i2400m/control.c:452:17: warning: cast to > > > restricted __le32 > > > drivers/staging//wimax/i2400m/control.c:452:17: warning: cast to > > > restricted __le32 > > > drivers/staging//wimax/i2400m/op-rfkill.c:159:14: warning: cast to > > > restricted __le32 > > > drivers/staging//wimax/i2400m/op-rfkill.c:160:14: warning: cast to > > > restricted __le32 > > > > > > Signed-off-by: Muhammad Usama Anjum > > > --- > > > drivers/staging/wimax/i2400m/control.c | 4 ++-- > > > drivers/staging/wimax/i2400m/op-rfkill.c | 4 ++-- > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/staging/wimax/i2400m/control.c > > > b/drivers/staging/wimax/i2400m/control.c > > > index 1e270b2101e8..b6b2788af162 100644 > > > --- a/drivers/staging/wimax/i2400m/control.c > > > +++ b/drivers/staging/wimax/i2400m/control.c > > > @@ -452,8 +452,8 @@ void i2400m_report_state_parse_tlv(struct i2400m > > > *i2400m, > > > d_printf(2, dev, "%s: RF status TLV " > > > "found (0x%04x), sw 0x%02x hw 0x%02x\n", > > > tag, I2400M_TLV_RF_STATUS, > > > -le32_to_cpu(rfss->sw_rf_switch), > > > -le32_to_cpu(rfss->hw_rf_switch)); > > > +rfss->sw_rf_switch, > > > +rfss->hw_rf_switch); > > > > What do you mean by "one byte"? This is a le32 sized variable, right? > > If not, why isn't the le32_to_cpu() call complaining? > > These two variables are of type _u8, one byte. > __u8 sw_rf_switch; > __u8 hw_rf_switch; > They aren't of type __le32. le32_to_cpu() macro should have > complained. But it isn't complaining. It seems like whatever we pass > to this macro, it casts it to __le32 forcefully (it seems like wrong). > So we'll never get any complain from this macro directly. But we are > getting complain from the sparse. > > For big endian: > #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x)) > For little endian: > #define __le32_to_cpu(x) ((__force __u32)(__le32)(x)) Right, it seems this driver has been broken on big-endian ever since it was first merged in 2008. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: atomisp: reduce kernel stack usage
From: Arnd Bergmann
The atomisp_set_fmt() function has multiple copies of the large
v4l2_format structure on its stack, resulting in a warning about
excessive stack usage in some rare randconfig builds.
drivers/staging/media/atomisp/pci/atomisp_cmd.c:5600:5: error: stack frame size
of 1084 bytes in function 'atomisp_set_fmt' [-Werror,-Wframe-larger-than=]
Of this structure, only three members in the 'fmt.pix' member are
used, so simplify it by using the smaller v4l2_pix_format structure
directly. This reduces the stack usage to 612 bytes, and it could
be reduced further by only storing the three members that are used.
Signed-off-by: Arnd Bergmann
---
.../staging/media/atomisp/pci/atomisp_cmd.c | 65 +--
.../staging/media/atomisp/pci/atomisp_cmd.h | 2 +-
.../staging/media/atomisp/pci/atomisp_ioctl.c | 2 +-
3 files changed, 33 insertions(+), 36 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 592ea990d4ca..3192c0716eb9 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -4837,7 +4837,7 @@ static void __atomisp_init_stream_info(u16 stream_index,
}
/* This function looks up the closest available resolution. */
-int atomisp_try_fmt(struct video_device *vdev, struct v4l2_format *f,
+int atomisp_try_fmt(struct video_device *vdev, struct v4l2_pix_format *f,
bool *res_overflow)
{
struct atomisp_device *isp = video_get_drvdata(vdev);
@@ -4859,18 +4859,18 @@ int atomisp_try_fmt(struct video_device *vdev, struct
v4l2_format *f,
return -EINVAL;
stream_index = atomisp_source_pad_to_stream_id(asd, source_pad);
- fmt = atomisp_get_format_bridge(f->fmt.pix.pixelformat);
+ fmt = atomisp_get_format_bridge(f->pixelformat);
if (!fmt) {
dev_err(isp->dev, "unsupported pixelformat!\n");
fmt = atomisp_output_fmts;
}
- if (f->fmt.pix.width <= 0 || f->fmt.pix.height <= 0)
+ if (f->width <= 0 || f->height <= 0)
return -EINVAL;
snr_mbus_fmt->code = fmt->mbus_code;
- snr_mbus_fmt->width = f->fmt.pix.width;
- snr_mbus_fmt->height = f->fmt.pix.height;
+ snr_mbus_fmt->width = f->width;
+ snr_mbus_fmt->height = f->height;
__atomisp_init_stream_info(stream_index, stream_info);
@@ -4892,7 +4892,7 @@ int atomisp_try_fmt(struct video_device *vdev, struct
v4l2_format *f,
return -EINVAL;
}
- f->fmt.pix.pixelformat = fmt->pixelformat;
+ f->pixelformat = fmt->pixelformat;
/*
* If the format is jpeg or custom RAW, then the width and height will
@@ -4900,17 +4900,17 @@ int atomisp_try_fmt(struct video_device *vdev, struct
v4l2_format *f,
* the below conditions. So just assign to what is being returned from
* the sensor driver.
*/
- if (f->fmt.pix.pixelformat == V4L2_PIX_FMT_JPEG ||
- f->fmt.pix.pixelformat == V4L2_PIX_FMT_CUSTOM_M10MO_RAW) {
- f->fmt.pix.width = snr_mbus_fmt->width;
- f->fmt.pix.height = snr_mbus_fmt->height;
+ if (f->pixelformat == V4L2_PIX_FMT_JPEG ||
+ f->pixelformat == V4L2_PIX_FMT_CUSTOM_M10MO_RAW) {
+ f->width = snr_mbus_fmt->width;
+ f->height = snr_mbus_fmt->height;
return 0;
}
- if (snr_mbus_fmt->width < f->fmt.pix.width
- && snr_mbus_fmt->height < f->fmt.pix.height) {
- f->fmt.pix.width = snr_mbus_fmt->width;
- f->fmt.pix.height = snr_mbus_fmt->height;
+ if (snr_mbus_fmt->width < f->width
+ && snr_mbus_fmt->height < f->height) {
+ f->width = snr_mbus_fmt->width;
+ f->height = snr_mbus_fmt->height;
/* Set the flag when resolution requested is
* beyond the max value supported by sensor
*/
@@ -4919,12 +4919,10 @@ int atomisp_try_fmt(struct video_device *vdev, struct
v4l2_format *f,
}
/* app vs isp */
- f->fmt.pix.width = rounddown(
- clamp_t(u32, f->fmt.pix.width,
ATOM_ISP_MIN_WIDTH,
- ATOM_ISP_MAX_WIDTH),
ATOM_ISP_STEP_WIDTH);
- f->fmt.pix.height = rounddown(
- clamp_t(u32, f->fmt.pix.height,
ATOM_ISP_MIN_HEIGHT,
- ATOM_ISP_MAX_HEIGHT),
ATOM_ISP_STEP_HEIGHT);
+ f->width = rounddown(clamp_t(u32, f->width, ATOM_ISP_MIN_WIDTH,
+ATOM_ISP_MAX_WIDTH), ATOM_ISP_STEP_WIDTH);
+ f-&
Re: [PATCH] staging: vt665x: fix alignment constraints
On Tue, Mar 16, 2021 at 7:17 PM Edmundo Carmona Antoranz
wrote:
>
> Removing 2 instances of alignment warnings
>
> drivers/staging/vt6655/rxtx.h:153:1: warning: alignment 1 of ‘struct vnt_cts’
> is less than 2 [-Wpacked-not-aligned]
> drivers/staging/vt6655/rxtx.h:163:1: warning: alignment 1 of ‘struct
> vnt_cts_fb’ is less than 2 [-Wpacked-not-aligned]
>
> The root cause seems to be that _because_ struct ieee80211_cts is marked as
> __aligned(2),
> this requires any encapsulating struct to also have an alignment of 2.
>
> Fixes: 2faf12c57efe ("staging: vt665x: fix alignment constraints")
> Signed-off-by: Edmundo Carmona Antoranz
Reviewed-by: Arnd Bergmann
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging/rtl8192u: avoid Wempty-body warning
From: Arnd Bergmann
This driver has a few disabled diagnostics, which can probably
just get removed, or might still be helpful:
drivers/staging/rtl8192u/r8192U_core.c: In function 'rtl8192_set_rxconf':
drivers/staging/rtl8192u/r8192U_core.c:767:45: error: suggest braces around
empty body in an 'if' statement [-Werror=empty-body]
767 | DMESG("NIC in promisc mode");
| ^
drivers/staging/rtl8192u/r8192U_core.c: In function
'rtl819xusb_rx_command_packet':
drivers/staging/rtl8192u/r8192U_core.c:883:80: error: suggest braces around
empty body in an 'if' statement [-Werror=empty-body]
883 | DMESG("rxcommandpackethandle819xusb: It is a command
packet\n");
|
^
Changing the empty macro to no_printk() to shut up the compiler warnings
and add format string checking.
Signed-off-by: Arnd Bergmann
---
drivers/staging/rtl8192u/r8192U.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8192u/r8192U.h
b/drivers/staging/rtl8192u/r8192U.h
index ec33fb9122e9..4013107cd93a 100644
--- a/drivers/staging/rtl8192u/r8192U.h
+++ b/drivers/staging/rtl8192u/r8192U.h
@@ -46,9 +46,9 @@
#define KEY_BUF_SIZE5
#defineRX_SMOOTH_FACTOR20
-#define DMESG(x, a...)
-#define DMESGW(x, a...)
-#define DMESGE(x, a...)
+#define DMESG(x, a...) no_printk(x, ##a)
+#define DMESGW(x, a...) no_printk(x, ##a)
+#define DMESGE(x, a...) no_printk(x, ##a)
extern u32 rt_global_debug_component;
#define RT_TRACE(component, x, args...) \
do {\
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging/nvec:: avoid Wempty-body warning
From: Arnd Bergmann
This driver has a few disabled diagnostics, which can probably
just get removed, or might still be helpful:
drivers/staging/nvec/nvec_ps2.c: In function 'nvec_ps2_notifier':
drivers/staging/nvec/nvec_ps2.c:94:77: error: suggest braces around empty body
in an 'if' statement [-Werror=empty-body]
94 | NVEC_PHD("unhandled mouse event: ", msg, msg[1]
+ 2);
Changing the empty macro to the usual 'do {} while (0)' at least
shuts up the compiler warnings.
Signed-off-by: Arnd Bergmann
---
drivers/staging/nvec/nvec_ps2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/nvec/nvec_ps2.c b/drivers/staging/nvec/nvec_ps2.c
index 157009015c3b..06041c7f7d4f 100644
--- a/drivers/staging/nvec/nvec_ps2.c
+++ b/drivers/staging/nvec/nvec_ps2.c
@@ -28,7 +28,7 @@
print_hex_dump(KERN_DEBUG, str, DUMP_PREFIX_NONE, \
16, 1, buf, len, false)
#else
-#define NVEC_PHD(str, buf, len)
+#define NVEC_PHD(str, buf, len) do { } while (0)
#endif
enum ps2_subcmds {
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/5] staging: vchiq: convert compat bulk transfer
Split out the ioctl implementation for VCHIQ_IOC_QUEUE_BULK_TRANSMIT
into a separate function so it can be shared with the compat
implementation.
Here, the input data is converted separately in the compat
handler, while the output data is passed as a __user pointer
to thec vchiq_queue_bulk_transfer->mode word that is
compatible.
Signed-off-by: Arnd Bergmann
---
.../interface/vchiq_arm/vchiq_arm.c | 220 +-
1 file changed, 109 insertions(+), 111 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index cbe9583a0114..50af7f4a1b7c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -938,6 +938,95 @@ static int vchiq_ioc_dequeue_message(struct vchiq_instance
*instance,
return ret;
}
+static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
+ struct vchiq_queue_bulk_transfer *args,
+ enum vchiq_bulk_dir dir,
+ enum vchiq_bulk_mode __user *mode)
+{
+ struct vchiq_service *service;
+ struct bulk_waiter_node *waiter = NULL;
+ int status = 0;
+ int ret;
+
+ service = find_service_for_instance(instance, args->handle);
+ if (!service)
+ return -EINVAL;
+
+ if (args->mode == VCHIQ_BULK_MODE_BLOCKING) {
+ waiter = kzalloc(sizeof(struct bulk_waiter_node),
+ GFP_KERNEL);
+ if (!waiter) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ args->userdata = &waiter->bulk_waiter;
+ } else if (args->mode == VCHIQ_BULK_MODE_WAITING) {
+ mutex_lock(&instance->bulk_waiter_list_mutex);
+ list_for_each_entry(waiter, &instance->bulk_waiter_list,
+ list) {
+ if (waiter->pid == current->pid) {
+ list_del(&waiter->list);
+ break;
+ }
+ }
+ mutex_unlock(&instance->bulk_waiter_list_mutex);
+ if (!waiter) {
+ vchiq_log_error(vchiq_arm_log_level,
+ "no bulk_waiter found for pid %d",
+ current->pid);
+ ret = -ESRCH;
+ goto out;
+ }
+ vchiq_log_info(vchiq_arm_log_level,
+ "found bulk_waiter %pK for pid %d", waiter,
+ current->pid);
+ args->userdata = &waiter->bulk_waiter;
+ }
+
+ status = vchiq_bulk_transfer(args->handle, args->data, args->size,
+args->userdata, args->mode, dir);
+
+ if (!waiter) {
+ ret = 0;
+ goto out;
+ }
+
+ if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
+ !waiter->bulk_waiter.bulk) {
+ if (waiter->bulk_waiter.bulk) {
+ /* Cancel the signal when the transfer
+ ** completes. */
+ spin_lock(&bulk_waiter_spinlock);
+ waiter->bulk_waiter.bulk->userdata = NULL;
+ spin_unlock(&bulk_waiter_spinlock);
+ }
+ kfree(waiter);
+ ret = 0;
+ } else {
+ const enum vchiq_bulk_mode mode_waiting =
+ VCHIQ_BULK_MODE_WAITING;
+ waiter->pid = current->pid;
+ mutex_lock(&instance->bulk_waiter_list_mutex);
+ list_add(&waiter->list, &instance->bulk_waiter_list);
+ mutex_unlock(&instance->bulk_waiter_list_mutex);
+ vchiq_log_info(vchiq_arm_log_level,
+ "saved bulk_waiter %pK for pid %d",
+ waiter, current->pid);
+
+ ret = put_user(mode_waiting, mode);
+ }
+out:
+ unlock_service(service);
+ if (ret)
+ return ret;
+ else if (status == VCHIQ_ERROR)
+ return -EIO;
+ else if (status == VCHIQ_RETRY)
+ return -EINTR;
+ return 0;
+}
+
/
*
* vchiq_ioctl
@@ -1118,90 +1207,20 @@ vchiq_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
case VCHIQ_IOC_QUEUE_BULK_TRANSMIT:
case VCHIQ_IOC_QUEUE_BULK_RECEIVE: {
struct vchiq_queue_bulk_transfer args;
- struct bulk_waiter_node *waiter =
[PATCH 1/5] staging: vchiq: rework compat handling
The compat handlers for VCHIQ_IOC_QUEUE_MESSAGE32 and
VCHIQ_IOC_GET_CONFIG32 can simply call the underlying implementations
that are already separate functions rather than using copy_in_user to
simulate the native 64-bit interface for the full ioctl handler.
vchiq_ioc_queue_message gets a small update to the calling
conventions to simplify the compat version by directly
returning a normal errno value.
Signed-off-by: Arnd Bergmann
---
.../interface/vchiq_arm/vchiq_arm.c | 109 +-
1 file changed, 56 insertions(+), 53 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index d4d811884861..56a38bec848a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -765,12 +765,13 @@ static ssize_t vchiq_ioc_copy_element_data(void *context,
void *dest,
* vchiq_ioc_queue_message
*
**/
-static enum vchiq_status
+static int
vchiq_ioc_queue_message(unsigned int handle,
struct vchiq_element *elements,
unsigned long count)
{
struct vchiq_io_copy_callback_context context;
+ enum vchiq_status status = VCHIQ_SUCCESS;
unsigned long i;
size_t total_size = 0;
@@ -785,8 +786,14 @@ vchiq_ioc_queue_message(unsigned int handle,
total_size += elements[i].size;
}
- return vchiq_queue_message(handle, vchiq_ioc_copy_element_data,
- &context, total_size);
+ status = vchiq_queue_message(handle, vchiq_ioc_copy_element_data,
+&context, total_size);
+
+ if (status == VCHIQ_ERROR)
+ return -EIO;
+ else if (status == VCHIQ_RETRY)
+ return -EINTR;
+ return 0;
}
/
@@ -1020,9 +1027,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
if (copy_from_user(elements, args.elements,
args.count * sizeof(struct vchiq_element)) == 0)
- status = vchiq_ioc_queue_message
- (args.handle,
- elements, args.count);
+ ret = vchiq_ioc_queue_message(args.handle,
elements,
+ args.count);
else
ret = -EFAULT;
} else {
@@ -1550,55 +1556,53 @@ struct vchiq_queue_message32 {
static long
vchiq_compat_ioctl_queue_message(struct file *file,
unsigned int cmd,
-unsigned long arg)
+struct vchiq_queue_message32 __user *arg)
{
- struct vchiq_queue_message __user *args;
- struct vchiq_element __user *elements;
+ struct vchiq_queue_message args;
struct vchiq_queue_message32 args32;
- unsigned int count;
-
- if (copy_from_user(&args32,
- (struct vchiq_queue_message32 __user *)arg,
- sizeof(args32)))
- return -EFAULT;
-
- args = compat_alloc_user_space(sizeof(*args) +
- (sizeof(*elements) * MAX_ELEMENTS));
+ struct vchiq_service *service;
+ int ret;
- if (!args)
+ if (copy_from_user(&args32, arg, sizeof(args32)))
return -EFAULT;
- if (put_user(args32.handle, &args->handle) ||
- put_user(args32.count, &args->count) ||
- put_user(compat_ptr(args32.elements), &args->elements))
- return -EFAULT;
+ args = (struct vchiq_queue_message) {
+ .handle = args32.handle,
+ .count= args32.count,
+ .elements = compat_ptr(args32.elements),
+ };
if (args32.count > MAX_ELEMENTS)
return -EINVAL;
- if (args32.elements && args32.count) {
- struct vchiq_element32 tempelement32[MAX_ELEMENTS];
+ service = find_service_for_instance(file->private_data, args.handle);
+ if (!service)
+ return -EINVAL;
- elements = (struct vchiq_element __user *)(args + 1);
+ if (args32.elements && args32.count) {
+ struct vchiq_element32 element32[MAX_ELEMENTS];
+ struct vchiq_element elements[MAX_ELEMENTS];
+ unsigned int count;
- if (copy_from_user(&tempelement32,
- compat_ptr(args32.elements),
- sizeof(tempelement32)
[PATCH 3/5] staging: vchiq: convert compat dequeue_message
Split out the ioctl implementation for VCHIQ_IOC_DEQUEUE_MESSAGE
into a separate function so it can be shared with the compat
implementation.
Signed-off-by: Arnd Bergmann
---
.../interface/vchiq_arm/vchiq_arm.c | 180 +-
1 file changed, 92 insertions(+), 88 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 1404a5a0c7b0..cbe9583a0114 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -858,6 +858,86 @@ static int vchiq_ioc_create_service(struct vchiq_instance
*instance,
return 0;
}
+static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
+struct vchiq_dequeue_message *args)
+{
+ struct user_service *user_service;
+ struct vchiq_service *service;
+ struct vchiq_header *header;
+ int ret;
+
+ DEBUG_INITIALISE(g_state.local)
+ DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+ service = find_service_for_instance(instance, args->handle);
+ if (!service)
+ return -EINVAL;
+
+ user_service = (struct user_service *)service->base.userdata;
+ if (user_service->is_vchi == 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ spin_lock(&msg_queue_spinlock);
+ if (user_service->msg_remove == user_service->msg_insert) {
+ if (!args->blocking) {
+ spin_unlock(&msg_queue_spinlock);
+ DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+ ret = -EWOULDBLOCK;
+ goto out;
+ }
+ user_service->dequeue_pending = 1;
+ ret = 0;
+ do {
+ spin_unlock(&msg_queue_spinlock);
+ DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+ if (wait_for_completion_interruptible(
+ &user_service->insert_event)) {
+ vchiq_log_info(vchiq_arm_log_level,
+ "DEQUEUE_MESSAGE interrupted");
+ ret = -EINTR;
+ break;
+ }
+ spin_lock(&msg_queue_spinlock);
+ } while (user_service->msg_remove ==
+ user_service->msg_insert);
+
+ if (ret)
+ goto out;
+ }
+
+ BUG_ON((int)(user_service->msg_insert -
+ user_service->msg_remove) < 0);
+
+ header = user_service->msg_queue[user_service->msg_remove &
+ (MSG_QUEUE_SIZE - 1)];
+ user_service->msg_remove++;
+ spin_unlock(&msg_queue_spinlock);
+
+ complete(&user_service->remove_event);
+ if (!header) {
+ ret = -ENOTCONN;
+ } else if (header->size <= args->bufsize) {
+ /* Copy to user space if msgbuf is not NULL */
+ if (!args->buf || (copy_to_user((void __user *)args->buf,
+ header->data, header->size) == 0)) {
+ ret = header->size;
+ vchiq_release_message(service->handle, header);
+ } else
+ ret = -EFAULT;
+ } else {
+ vchiq_log_error(vchiq_arm_log_level,
+ "header %pK: bufsize %x < size %x",
+ header, args->bufsize, header->size);
+ WARN(1, "invalid size\n");
+ ret = -EMSGSIZE;
+ }
+ DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+out:
+ unlock_service(service);
+ return ret;
+}
+
/
*
* vchiq_ioctl
@@ -1287,84 +1367,14 @@ vchiq_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
case VCHIQ_IOC_DEQUEUE_MESSAGE: {
struct vchiq_dequeue_message args;
- struct user_service *user_service;
- struct vchiq_header *header;
- DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
if (copy_from_user(&args, (const void __user *)arg,
sizeof(args))) {
ret = -EFAULT;
break;
}
- service = find_service_for_instance(instance, args.handle);
- if (!service) {
- ret = -EINVAL;
- break;
- }
- user_service = (struct user_service *)service->base.userdata;
- if (user_service->is_vchi == 0) {
- ret = -EINVAL;
-
[PATCH 2/5] staging: vchiq: convert compat create_service
Split out the ioctl implementation for VCHIQ_IOC_CREATE_SERVICE
into a separate function so it can be shared with the compat
implementation.
Signed-off-by: Arnd Bergmann
---
.../interface/vchiq_arm/vchiq_arm.c | 189 +-
1 file changed, 89 insertions(+), 100 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 56a38bec848a..1404a5a0c7b0 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -796,6 +796,68 @@ vchiq_ioc_queue_message(unsigned int handle,
return 0;
}
+static int vchiq_ioc_create_service(struct vchiq_instance *instance,
+ struct vchiq_create_service *args)
+{
+ struct user_service *user_service = NULL;
+ struct vchiq_service *service;
+ enum vchiq_status status = VCHIQ_SUCCESS;
+ void *userdata;
+ int srvstate;
+
+ user_service = kmalloc(sizeof(*user_service), GFP_KERNEL);
+ if (!user_service)
+ return -ENOMEM;
+
+ if (args->is_open) {
+ if (!instance->connected) {
+ kfree(user_service);
+ return -ENOTCONN;
+ }
+ srvstate = VCHIQ_SRVSTATE_OPENING;
+ } else {
+ srvstate = instance->connected ?
+VCHIQ_SRVSTATE_LISTENING : VCHIQ_SRVSTATE_HIDDEN;
+ }
+
+ userdata = args->params.userdata;
+ args->params.callback = service_callback;
+ args->params.userdata = user_service;
+ service = vchiq_add_service_internal(instance->state, &args->params,
+srvstate, instance,
+user_service_free);
+
+ if (!service) {
+ kfree(user_service);
+ return -EEXIST;
+ }
+
+ user_service->service = service;
+ user_service->userdata = userdata;
+ user_service->instance = instance;
+ user_service->is_vchi = (args->is_vchi != 0);
+ user_service->dequeue_pending = 0;
+ user_service->close_pending = 0;
+ user_service->message_available_pos = instance->completion_remove - 1;
+ user_service->msg_insert = 0;
+ user_service->msg_remove = 0;
+ init_completion(&user_service->insert_event);
+ init_completion(&user_service->remove_event);
+ init_completion(&user_service->close_event);
+
+ if (args->is_open) {
+ status = vchiq_open_service_internal(service, instance->pid);
+ if (status != VCHIQ_SUCCESS) {
+ vchiq_remove_service(service->handle);
+ return (status == VCHIQ_RETRY) ?
+ -EINTR : -EIO;
+ }
+ }
+ args->handle = service->handle;
+
+ return 0;
+}
+
/
*
* vchiq_ioctl
@@ -868,85 +930,22 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
break;
case VCHIQ_IOC_CREATE_SERVICE: {
+ struct vchiq_create_service __user *argp;
struct vchiq_create_service args;
- struct user_service *user_service = NULL;
- void *userdata;
- int srvstate;
- if (copy_from_user(&args, (const void __user *)arg,
- sizeof(args))) {
+ argp = (void __user *)arg;
+ if (copy_from_user(&args, argp, sizeof(args))) {
ret = -EFAULT;
break;
}
- user_service = kmalloc(sizeof(*user_service), GFP_KERNEL);
- if (!user_service) {
- ret = -ENOMEM;
+ ret = vchiq_ioc_create_service(instance, &args);
+ if (ret < 0)
break;
- }
-
- if (args.is_open) {
- if (!instance->connected) {
- ret = -ENOTCONN;
- kfree(user_service);
- break;
- }
- srvstate = VCHIQ_SRVSTATE_OPENING;
- } else {
- srvstate =
-instance->connected ?
-VCHIQ_SRVSTATE_LISTENING :
-VCHIQ_SRVSTATE_HIDDEN;
- }
- userdata = args.params.userdata;
- args.params.callback = service_callback;
- args.params.userdata = user_service;
- service = vchiq_add_ser
[PATCH 0/5] staging: vchiq: stop using compat_alloc_user_space
This driver is one of only a few remaining files using compat_alloc_user_space() and copy_in_user() to implement the compat_ioctl handlers. Change it to be more like the other drivers, calling the underlying implementation directly, which is generally simpler and less error-prone. This is only build tested so far. Arnd Arnd Bergmann (5): staging: vchiq: rework compat handling staging: vchiq: convert compat create_service staging: vchiq: convert compat dequeue_message staging: vchiq: convert compat bulk transfer staging: vchiq: convert compat await_completion .../interface/vchiq_arm/vchiq_arm.c | 1194 - 1 file changed, 551 insertions(+), 643 deletions(-) -- 2.27.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/5] staging: vchiq: convert compat await_completion
Split out the ioctl implementation for VCHIQ_IOC_QUEUE_BULK_TRANSMIT
into a separate function so it can be shared with the compat
implementation.
This one is the trickiest conversion, as the compat implementation
is already quite different from the native one. By using a common
handler, the behavior is changed to be the same again: The
indirect __user pointer accesses are now handled through helper
functions that check for compat mode internally.
Signed-off-by: Arnd Bergmann
---
.../interface/vchiq_arm/vchiq_arm.c | 496 --
1 file changed, 205 insertions(+), 291 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 50af7f4a1b7c..bb0cc9cb96e9 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1027,6 +1027,193 @@ static int vchiq_irq_queue_bulk_tx_rx(struct
vchiq_instance *instance,
return 0;
}
+static inline int vchiq_get_user_ptr(void __user **buf, void __user *ubuf, int
index)
+{
+ compat_uptr_t ptr32;
+ int ret;
+
+ if (in_compat_syscall()) {
+ compat_uptr_t __user *uptr = ubuf;
+ ret = get_user(ptr32, &uptr[index]);
+ *buf = compat_ptr(ptr32);
+ } else {
+ void __user *__user *uptr = ubuf;
+ ret = get_user(buf, &uptr[index]);
+ }
+ return ret;
+}
+
+struct vchiq_completion_data32 {
+ enum vchiq_reason reason;
+ compat_uptr_t header;
+ compat_uptr_t service_userdata;
+ compat_uptr_t bulk_userdata;
+};
+
+static int vchiq_put_completion(struct vchiq_completion_data __user *buf,
+ struct vchiq_completion_data *completion,
+ int index)
+{
+ struct vchiq_completion_data32 __user *buf32 = (void __user *)buf;
+
+ if (in_compat_syscall()) {
+ struct vchiq_completion_data32 tmp = {
+ .reason = buf->reason,
+ .header = ptr_to_compat(buf->header),
+ .service_userdata =
ptr_to_compat(buf->service_userdata),
+ .bulk_userdata= ptr_to_compat(buf->bulk_userdata),
+ };
+ if (copy_to_user(&buf32[index], &tmp, sizeof(tmp)))
+ return -EFAULT;
+ } else {
+ if (copy_to_user(&buf[index], completion, sizeof(*completion)))
+ return -EFAULT;
+ }
+
+ return 0;
+}
+
+static int vchiq_ioc_await_completion(struct vchiq_instance *instance,
+ struct vchiq_await_completion *args,
+ int __user *msgbufcountp)
+{
+ int msgbufcount;
+ int remove;
+ int ret;
+
+ DEBUG_INITIALISE(g_state.local)
+
+ DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+ if (!instance->connected) {
+ return -ENOTCONN;
+ }
+
+ mutex_lock(&instance->completion_mutex);
+
+ DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+ while ((instance->completion_remove ==
+ instance->completion_insert)
+ && !instance->closing) {
+ int rc;
+
+ DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+ mutex_unlock(&instance->completion_mutex);
+ rc = wait_for_completion_interruptible(
+ &instance->insert_event);
+ mutex_lock(&instance->completion_mutex);
+ if (rc) {
+ DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+ vchiq_log_info(vchiq_arm_log_level,
+ "AWAIT_COMPLETION interrupted");
+ ret = -EINTR;
+ goto out;
+ }
+ }
+ DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+
+ msgbufcount = args->msgbufcount;
+ remove = instance->completion_remove;
+
+ for (ret = 0; ret < args->count; ret++) {
+ struct vchiq_completion_data *completion;
+ struct vchiq_service *service;
+ struct user_service *user_service;
+ struct vchiq_header *header;
+
+ if (remove == instance->completion_insert)
+ break;
+
+ completion = &instance->completions[
+ remove & (MAX_COMPLETIONS - 1)];
+
+ /*
+* A read memory barrier is needed to stop
+* prefetch of a stale completion record
+*/
+ rmb();
+
+ service = completion->service_userdata;
+ user_service = service->base.userdata;
+ comple
[PATCH 1/2] staging: vchiq: fix __user annotations
My earlier patches caused some new sparse warnings, but it turns out
that a number of those are actual bugs, or at least suspicous code.
Adding __user annotations to the data structures that are defined in
uapi headers helps avoid the new warnings, but that causes a different
set of warnings to show up, as some of these structures are used both
inside of the kernel and at the user interface but storing pointers to
different things there.
Duplicating the vchiq_service_params and vchiq_completion_data structures
in turn takes care of most of those, and then it turns out that there
is a 'data' pointer that can be any of a __user address, a dmd_addr_t
and a kernel pointer in vmalloc space at times.
I'm trying to annotate these as best I can without changing behavior,
but there still seems to be a serious bug when user space passes
a valid vmalloc space address instead of a user pointer. Adding
comments in the code there, and leaving the warnings in place that
seem to correspond to actual bugs.
Signed-off-by: Arnd Bergmann
---
.../include/linux/raspberrypi/vchiq.h | 11 ++-
.../interface/vchiq_arm/vchiq_2835_arm.c | 2 +-
.../interface/vchiq_arm/vchiq_arm.c | 95 ---
.../interface/vchiq_arm/vchiq_core.c | 19 ++--
.../interface/vchiq_arm/vchiq_core.h | 10 +-
.../interface/vchiq_arm/vchiq_ioctl.h | 29 --
6 files changed, 106 insertions(+), 60 deletions(-)
diff --git a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
index 18d63df368c4..fefc664eefcf 100644
--- a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
+++ b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
@@ -62,7 +62,14 @@ struct vchiq_service_base {
void *userdata;
};
-struct vchiq_service_params {
+struct vchiq_completion_data_kernel {
+ enum vchiq_reason reason;
+ struct vchiq_header *header;
+ void *service_userdata;
+ void *bulk_userdata;
+};
+
+struct vchiq_service_params_kernel {
int fourcc;
enum vchiq_status (*callback)(enum vchiq_reason reason,
struct vchiq_header *header,
@@ -79,7 +86,7 @@ extern enum vchiq_status vchiq_initialise(struct
vchiq_instance **pinstance);
extern enum vchiq_status vchiq_shutdown(struct vchiq_instance *instance);
extern enum vchiq_status vchiq_connect(struct vchiq_instance *instance);
extern enum vchiq_status vchiq_open_service(struct vchiq_instance *instance,
- const struct vchiq_service_params *params,
+ const struct vchiq_service_params_kernel *params,
unsigned int *pservice);
extern enum vchiq_status vchiq_close_service(unsigned int service);
extern enum vchiq_status vchiq_use_service(unsigned int service);
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 5ed36d557014..7dc7441db592 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -229,7 +229,7 @@ vchiq_prepare_bulk_data(struct vchiq_bulk *bulk, void
*offset, int size,
if (!pagelistinfo)
return VCHIQ_ERROR;
- bulk->data = (void *)(unsigned long)pagelistinfo->dma_addr;
+ bulk->data = pagelistinfo->dma_addr;
/*
* Store the pagelistinfo address in remote_data,
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index bb0cc9cb96e9..4fb19e68eadf 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -53,7 +53,7 @@ int vchiq_susp_log_level = VCHIQ_LOG_ERROR;
struct user_service {
struct vchiq_service *service;
- void *userdata;
+ void __user *userdata;
struct vchiq_instance *instance;
char is_vchi;
char dequeue_pending;
@@ -75,7 +75,7 @@ struct bulk_waiter_node {
struct vchiq_instance {
struct vchiq_state *state;
- struct vchiq_completion_data completions[MAX_COMPLETIONS];
+ struct vchiq_completion_data_kernel completions[MAX_COMPLETIONS];
int completion_insert;
int completion_remove;
struct completion insert_event;
@@ -273,7 +273,7 @@ EXPORT_SYMBOL(vchiq_connect);
static enum vchiq_status vchiq_add_service(
struct vchiq_instance *instance,
- const struct vchiq_service_params *params,
+ const struct vchiq_service_params_kernel *params,
unsigned int *phandle)
{
enum vchiq_status status;
@@ -311,7 +311,7 @@ static enum vchiq_status vchiq_add_service(
enum vchiq_status vchiq_open_service(
struct vchiq_instance *ins
[PATCH 2/2] staging: vchiq: avoid mixing kernel and user pointers
As found earlier, there is a problem in the create_pagelist() function
that takes a pointer argument that either points into vmalloc space or
into user space, with the pointer value controlled by user space allowing
a malicious user to trick the driver into accessing the kernel instead.
Avoid this problem by adding another function argument and passing
kernel pointers separately from user pointers. This makes it possible
to rely on sparse to point out invalid conversions, and it prevents
user space from faking a kernel pointer.
Signed-off-by: Arnd Bergmann
---
.../interface/vchiq_arm/vchiq_2835_arm.c | 22 +++
.../interface/vchiq_arm/vchiq_arm.c | 14 +++-
.../interface/vchiq_arm/vchiq_core.c | 10 +
.../interface/vchiq_arm/vchiq_core.h | 6 ++---
4 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 7dc7441db592..8782ebe0b39a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -70,7 +70,7 @@ static irqreturn_t
vchiq_doorbell_irq(int irq, void *dev_id);
static struct vchiq_pagelist_info *
-create_pagelist(char __user *buf, size_t count, unsigned short type);
+create_pagelist(char *buf, char __user *ubuf, size_t count, unsigned short
type);
static void
free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
@@ -216,12 +216,12 @@ remote_event_signal(struct remote_event *event)
}
enum vchiq_status
-vchiq_prepare_bulk_data(struct vchiq_bulk *bulk, void *offset, int size,
- int dir)
+vchiq_prepare_bulk_data(struct vchiq_bulk *bulk, void *offset,
+ void __user *uoffset, int size, int dir)
{
struct vchiq_pagelist_info *pagelistinfo;
- pagelistinfo = create_pagelist((char __user *)offset, size,
+ pagelistinfo = create_pagelist(offset, uoffset, size,
(dir == VCHIQ_BULK_RECEIVE)
? PAGELIST_READ
: PAGELIST_WRITE);
@@ -304,7 +304,8 @@ cleanup_pagelistinfo(struct vchiq_pagelist_info
*pagelistinfo)
*/
static struct vchiq_pagelist_info *
-create_pagelist(char __user *buf, size_t count, unsigned short type)
+create_pagelist(char *buf, char __user *ubuf,
+ size_t count, unsigned short type)
{
struct pagelist *pagelist;
struct vchiq_pagelist_info *pagelistinfo;
@@ -320,7 +321,10 @@ create_pagelist(char __user *buf, size_t count, unsigned
short type)
if (count >= INT_MAX - PAGE_SIZE)
return NULL;
- offset = ((unsigned int)(unsigned long)buf & (PAGE_SIZE - 1));
+ if (buf)
+ offset = (uintptr_t)buf & (PAGE_SIZE - 1);
+ else
+ offset = (uintptr_t)ubuf & (PAGE_SIZE - 1);
num_pages = DIV_ROUND_UP(count + offset, PAGE_SIZE);
if (num_pages > (SIZE_MAX - sizeof(struct pagelist) -
@@ -368,14 +372,14 @@ create_pagelist(char __user *buf, size_t count, unsigned
short type)
pagelistinfo->scatterlist = scatterlist;
pagelistinfo->scatterlist_mapped = 0;
- if (is_vmalloc_addr((void __force *)buf)) {
+ if (buf) {
unsigned long length = count;
unsigned int off = offset;
for (actual_pages = 0; actual_pages < num_pages;
actual_pages++) {
struct page *pg =
- vmalloc_to_page((void __force *)(buf +
+ vmalloc_to_page((buf +
(actual_pages * PAGE_SIZE)));
size_t bytes = PAGE_SIZE - off;
@@ -393,7 +397,7 @@ create_pagelist(char __user *buf, size_t count, unsigned
short type)
/* do not try and release vmalloc pages */
} else {
actual_pages = pin_user_pages_fast(
- (unsigned long)buf & PAGE_MASK,
+ (unsigned long)ubuf & PAGE_MASK,
num_pages,
type == PAGELIST_READ,
pages);
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 4fb19e68eadf..590415561b73 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -360,8 +360,8 @@ vchiq_bulk_transmit(unsigned int handle, const void *data,
case VCHIQ_BULK_MODE_NOCALLBACK:
case VCHIQ_BULK_MODE_CALLBACK:
Re: [PATCH 1/2] staging: vchiq: fix __user annotations
On Wed, Sep 23, 2020 at 7:44 AM Greg Kroah-Hartman
wrote:
> and so on...
>
> Care to try a v2?
I had a look now and found the problem, as there are two drivers that I did not
have enabled but that need a trivial change to match my other modifications.
I'll send the new version in a bit, changes below for reference.
Arnd
index 292fcee9d6f2..d567a2e3f70c 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -122,7 +122,7 @@ static int
vc_vchi_audio_init(struct vchiq_instance *vchiq_instance,
struct bcm2835_audio_instance *instance)
{
- struct vchiq_service_params params = {
+ struct vchiq_service_params_kernel params = {
.version= VC_AUDIOSERV_VER,
.version_min= VC_AUDIOSERV_MIN_VER,
.fourcc = VCHIQ_MAKE_FOURCC('A', 'U', 'D', 'S'),
diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
index e798d494f00f..3a4202551cfc 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
@@ -1858,7 +1858,7 @@ int vchiq_mmal_init(struct vchiq_mmal_instance
**out_instance)
int status;
struct vchiq_mmal_instance *instance;
static struct vchiq_instance *vchiq_instance;
- struct vchiq_service_params params = {
+ struct vchiq_service_params_kernel params = {
.version= VC_MMAL_VER,
.version_min= VC_MMAL_MIN_VER,
.fourcc = VCHIQ_MAKE_FOURCC('m', 'm', 'a', 'l'),
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/2] staging: vchiq: fix __user annotations
My earlier patches caused some new sparse warnings, but it turns out
that a number of those are actual bugs, or at least suspicous code.
Adding __user annotations to the data structures that are defined in
uapi headers helps avoid the new warnings, but that causes a different
set of warnings to show up, as some of these structures are used both
inside of the kernel and at the user interface but storing pointers to
different things there.
Duplicating the vchiq_service_params and vchiq_completion_data structures
in turn takes care of most of those, and then it turns out that there
is a 'data' pointer that can be any of a __user address, a dmd_addr_t
and a kernel pointer in vmalloc space at times.
I'm trying to annotate these as best I can without changing behavior,
but there still seems to be a serious bug when user space passes
a valid vmalloc space address instead of a user pointer. Adding
comments in the code there, and leaving the warnings in place that
seem to correspond to actual bugs.
Signed-off-by: Arnd Bergmann
---
.../bcm2835-audio/bcm2835-vchiq.c | 2 +-
.../include/linux/raspberrypi/vchiq.h | 11 ++-
.../interface/vchiq_arm/vchiq_2835_arm.c | 2 +-
.../interface/vchiq_arm/vchiq_arm.c | 95 ---
.../interface/vchiq_arm/vchiq_core.c | 19 ++--
.../interface/vchiq_arm/vchiq_core.h | 10 +-
.../interface/vchiq_arm/vchiq_ioctl.h | 29 --
.../vc04_services/vchiq-mmal/mmal-vchiq.c | 2 +-
8 files changed, 108 insertions(+), 62 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index 292fcee9d6f2..d567a2e3f70c 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -122,7 +122,7 @@ static int
vc_vchi_audio_init(struct vchiq_instance *vchiq_instance,
struct bcm2835_audio_instance *instance)
{
- struct vchiq_service_params params = {
+ struct vchiq_service_params_kernel params = {
.version= VC_AUDIOSERV_VER,
.version_min= VC_AUDIOSERV_MIN_VER,
.fourcc = VCHIQ_MAKE_FOURCC('A', 'U', 'D', 'S'),
diff --git a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
index 18d63df368c4..fefc664eefcf 100644
--- a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
+++ b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
@@ -62,7 +62,14 @@ struct vchiq_service_base {
void *userdata;
};
-struct vchiq_service_params {
+struct vchiq_completion_data_kernel {
+ enum vchiq_reason reason;
+ struct vchiq_header *header;
+ void *service_userdata;
+ void *bulk_userdata;
+};
+
+struct vchiq_service_params_kernel {
int fourcc;
enum vchiq_status (*callback)(enum vchiq_reason reason,
struct vchiq_header *header,
@@ -79,7 +86,7 @@ extern enum vchiq_status vchiq_initialise(struct
vchiq_instance **pinstance);
extern enum vchiq_status vchiq_shutdown(struct vchiq_instance *instance);
extern enum vchiq_status vchiq_connect(struct vchiq_instance *instance);
extern enum vchiq_status vchiq_open_service(struct vchiq_instance *instance,
- const struct vchiq_service_params *params,
+ const struct vchiq_service_params_kernel *params,
unsigned int *pservice);
extern enum vchiq_status vchiq_close_service(unsigned int service);
extern enum vchiq_status vchiq_use_service(unsigned int service);
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 5ed36d557014..7dc7441db592 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -229,7 +229,7 @@ vchiq_prepare_bulk_data(struct vchiq_bulk *bulk, void
*offset, int size,
if (!pagelistinfo)
return VCHIQ_ERROR;
- bulk->data = (void *)(unsigned long)pagelistinfo->dma_addr;
+ bulk->data = pagelistinfo->dma_addr;
/*
* Store the pagelistinfo address in remote_data,
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index bb0cc9cb96e9..4fb19e68eadf 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -53,7 +53,7 @@ int vchiq_susp_log_level = VCHIQ_LOG_ERROR;
struct user_service {
struct vchiq_service *service;
- void *userdata;
+ void __user *userdata;
struct vchiq_ins
[PATCH v2 2/2] staging: vchiq: avoid mixing kernel and user pointers
As found earlier, there is a problem in the create_pagelist() function
that takes a pointer argument that either points into vmalloc space or
into user space, with the pointer value controlled by user space allowing
a malicious user to trick the driver into accessing the kernel instead.
Avoid this problem by adding another function argument and passing
kernel pointers separately from user pointers. This makes it possible
to rely on sparse to point out invalid conversions, and it prevents
user space from faking a kernel pointer.
Signed-off-by: Arnd Bergmann
---
.../interface/vchiq_arm/vchiq_2835_arm.c | 22 +++
.../interface/vchiq_arm/vchiq_arm.c | 14 +++-
.../interface/vchiq_arm/vchiq_core.c | 10 +
.../interface/vchiq_arm/vchiq_core.h | 6 ++---
4 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 7dc7441db592..8782ebe0b39a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -70,7 +70,7 @@ static irqreturn_t
vchiq_doorbell_irq(int irq, void *dev_id);
static struct vchiq_pagelist_info *
-create_pagelist(char __user *buf, size_t count, unsigned short type);
+create_pagelist(char *buf, char __user *ubuf, size_t count, unsigned short
type);
static void
free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
@@ -216,12 +216,12 @@ remote_event_signal(struct remote_event *event)
}
enum vchiq_status
-vchiq_prepare_bulk_data(struct vchiq_bulk *bulk, void *offset, int size,
- int dir)
+vchiq_prepare_bulk_data(struct vchiq_bulk *bulk, void *offset,
+ void __user *uoffset, int size, int dir)
{
struct vchiq_pagelist_info *pagelistinfo;
- pagelistinfo = create_pagelist((char __user *)offset, size,
+ pagelistinfo = create_pagelist(offset, uoffset, size,
(dir == VCHIQ_BULK_RECEIVE)
? PAGELIST_READ
: PAGELIST_WRITE);
@@ -304,7 +304,8 @@ cleanup_pagelistinfo(struct vchiq_pagelist_info
*pagelistinfo)
*/
static struct vchiq_pagelist_info *
-create_pagelist(char __user *buf, size_t count, unsigned short type)
+create_pagelist(char *buf, char __user *ubuf,
+ size_t count, unsigned short type)
{
struct pagelist *pagelist;
struct vchiq_pagelist_info *pagelistinfo;
@@ -320,7 +321,10 @@ create_pagelist(char __user *buf, size_t count, unsigned
short type)
if (count >= INT_MAX - PAGE_SIZE)
return NULL;
- offset = ((unsigned int)(unsigned long)buf & (PAGE_SIZE - 1));
+ if (buf)
+ offset = (uintptr_t)buf & (PAGE_SIZE - 1);
+ else
+ offset = (uintptr_t)ubuf & (PAGE_SIZE - 1);
num_pages = DIV_ROUND_UP(count + offset, PAGE_SIZE);
if (num_pages > (SIZE_MAX - sizeof(struct pagelist) -
@@ -368,14 +372,14 @@ create_pagelist(char __user *buf, size_t count, unsigned
short type)
pagelistinfo->scatterlist = scatterlist;
pagelistinfo->scatterlist_mapped = 0;
- if (is_vmalloc_addr((void __force *)buf)) {
+ if (buf) {
unsigned long length = count;
unsigned int off = offset;
for (actual_pages = 0; actual_pages < num_pages;
actual_pages++) {
struct page *pg =
- vmalloc_to_page((void __force *)(buf +
+ vmalloc_to_page((buf +
(actual_pages * PAGE_SIZE)));
size_t bytes = PAGE_SIZE - off;
@@ -393,7 +397,7 @@ create_pagelist(char __user *buf, size_t count, unsigned
short type)
/* do not try and release vmalloc pages */
} else {
actual_pages = pin_user_pages_fast(
- (unsigned long)buf & PAGE_MASK,
+ (unsigned long)ubuf & PAGE_MASK,
num_pages,
type == PAGELIST_READ,
pages);
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 4fb19e68eadf..590415561b73 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -360,8 +360,8 @@ vchiq_bulk_transmit(unsigned int handle, const void *data,
case VCHIQ_BULK_MODE_NOCALLBACK:
case VCHIQ_BULK_MODE_CALLBACK:
Re: [PATCH] staging: vchiq: silence an uninitialized variable warning
On Wed, Sep 30, 2020 at 11:02 AM Dan Carpenter wrote:
>
> Smatch complains that "userdata" can be passed to vchiq_bulk_transfer()
> without being initialized. Smatch is correct, however, in that
> situation the "userdata" is not used so it doesn't cause a problem.
> Passing an uninitialized variable will trigger a UBSan warning at
> runtime so this warning is worth silencing by setting "userdata" to
> NULL.
>
> Fixes: a4367cd2b231 ("staging: vchiq: convert compat bulk transfer")
> Signed-off-by: Dan Carpenter
The change looks fine, but I wonder if it's actually worse and the
uninitialized pointer can end up getting copied back to user space
in the completion.
In either case, thanks for the fix!
Acked-by: Arnd Bergmann
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: atomisp: stop compiling compat_ioctl32 code
This is one of the last remaining users of compat_alloc_user_space()
and copy_in_user(), which are in the process of getting removed.
As of commit 57e6b6f2303e ("media: atomisp_fops.c: disable
atomisp_compat_ioctl32"), nothing in this file is actually getting used
as the only reference has been stubbed out.
Do the same thing here and stub out the implementation as well while
leaving it in place, with a comment explaining the problem.
Alternatively, the entire file could just be removed, since anyone
willing to restore the functionality can equally well just look up
the contents in the git history if needed.
Cc: Christoph Hellwig
Cc: Sakari Ailus
Cc: Hans Verkuil
Signed-off-by: Arnd Bergmann
---
.../staging/media/atomisp/pci/atomisp_compat_ioctl32.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c
b/drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c
index e5553df5bad4..bc6ef902a520 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c
@@ -15,7 +15,15 @@
*
*
*/
-#ifdef CONFIG_COMPAT
+
+/*
+ * The compat code is disabled for now, as compat_alloc_user_space()
+ * is in the process of getting removed. The compat_ioctl implementation
+ * here was already disabled in commit 57e6b6f2303e ("media: atomisp_fops.c:
+ * disable atomisp_compat_ioctl32"), so this is all dead code, but it
+ * is left for reference as long as something like it is in fact needed.
+ */
+#if 0 /* #ifdef CONFIG_COMPAT */
#include
#include
--
2.27.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] media: atomisp: remove compat_ioctl32 code
This is one of the last remaining users of compat_alloc_user_space()
and copy_in_user(), which are in the process of getting removed.
As of commit 57e6b6f2303e ("media: atomisp_fops.c: disable
atomisp_compat_ioctl32"), nothing in this file is actually getting used
as the only reference has been stubbed out.
Remove the entire file -- anyone willing to restore the functionality
can equally well just look up the contents in the git history if needed.
Cc: Sakari Ailus
Cc: Hans Verkuil
Suggested-by: Christoph Hellwig
Signed-off-by: Arnd Bergmann
---
This is the alternative approach for the patch, removing the already
dead code instead of just not compiling it.
---
drivers/staging/media/atomisp/Makefile|1 -
drivers/staging/media/atomisp/TODO|5 +
.../atomisp/pci/atomisp_compat_ioctl32.c | 1202 -
.../staging/media/atomisp/pci/atomisp_fops.c |8 +-
4 files changed, 8 insertions(+), 1208 deletions(-)
delete mode 100644 drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c
diff --git a/drivers/staging/media/atomisp/Makefile
b/drivers/staging/media/atomisp/Makefile
index 1dfad0dd02d0..3bbd4bf4408c 100644
--- a/drivers/staging/media/atomisp/Makefile
+++ b/drivers/staging/media/atomisp/Makefile
@@ -16,7 +16,6 @@ atomisp-objs += \
pci/atomisp_acc.o \
pci/atomisp_cmd.o \
pci/atomisp_compat_css20.o \
- pci/atomisp_compat_ioctl32.o \
pci/atomisp_csi2.o \
pci/atomisp_drvfs.o \
pci/atomisp_file.o \
diff --git a/drivers/staging/media/atomisp/TODO
b/drivers/staging/media/atomisp/TODO
index 6987bb2d32cf..2d1ef9eb262a 100644
--- a/drivers/staging/media/atomisp/TODO
+++ b/drivers/staging/media/atomisp/TODO
@@ -120,6 +120,11 @@ TODO
for this driver until the other work is done, as there will be a lot
of code churn until this driver becomes functional again.
+16. Fix private ioctls to not need a compat_ioctl handler for running
+32-bit tasks. The compat code has been removed because of bugs,
+and should not be needed for modern drivers. Fixing this properly
+unfortunately means an incompatible ABI change.
+
Limitations
===
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c
b/drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c
deleted file mode 100644
index e5553df5bad4..
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_ioctl32.c
+++ /dev/null
@@ -1,1202 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Support for Intel Camera Imaging ISP subsystem.
- *
- * Copyright (c) 2013 Intel Corporation. All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License version
- * 2 as published by the Free Software Foundation.
- *
- * 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.
- *
- *
- */
-#ifdef CONFIG_COMPAT
-#include
-
-#include
-
-#include "atomisp_internal.h"
-#include "atomisp_compat.h"
-#include "atomisp_ioctl.h"
-#include "atomisp_compat_ioctl32.h"
-
-/* Macros borrowed from v4l2-compat-ioctl32.c */
-
-#define get_user_cast(__x, __ptr) \
-({ \
- get_user(__x, (typeof(*__ptr) __user *)(__ptr));\
-})
-
-#define put_user_force(__x, __ptr) \
-({ \
- put_user((typeof(*__x) __force *)(__x), __ptr); \
-})
-
-/* Use the same argument order as copy_in_user */
-#define assign_in_user(to, from) \
-({ \
- typeof(*from) __assign_tmp; \
- \
- get_user_cast(__assign_tmp, from) || put_user(__assign_tmp, to);\
-})
-
-static int get_atomisp_histogram32(struct atomisp_histogram __user *kp,
- struct atomisp_histogram32 __user *up)
-{
- compat_uptr_t tmp;
-
- if (!access_ok(up, sizeof(struct atomisp_histogram32)) ||
- assign_in_user(&kp->num_elements, &up->num_elements) ||
- get_user(tmp, &up->data) ||
- put_user(compat_ptr(tmp), &kp->data))
- return -EFAULT;
-
- return 0;
-}
-
-static int put_atomisp_histogram32(struct atomisp_histogram __user *kp,
- struct atomisp_histogram32 __user *up)
-{
- void __user *tmp;
-
- if (!access_ok(up, sizeof(struct atomisp_hi
Re: [PATCH] media: atomisp: stop compiling compat_ioctl32 code
On Wed, Oct 7, 2020 at 4:18 PM Christoph Hellwig wrote: > > On Wed, Oct 07, 2020 at 04:16:39PM +0200, Arnd Bergmann wrote: > > Alternatively, the entire file could just be removed, since anyone > > willing to restore the functionality can equally well just look up > > the contents in the git history if needed. > > I suspect that is the right thing. Note that given that the driver > is only in staging anyway, the right thing to do would be to change > the ioctl ABI to be more compat friendly to start with. Ok, I sent that as v2 now. I wonder how many of those 56 ioctl commands in the driver are actually used in practice. Is there a public repository for the matching user space? Is it required to even use the driver or is it otherwise compatible with normal v4l2 applications? Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: wfx: avoid uninitialized variable use
From: Arnd Bergmann
Move the added check of the 'band' variable after the
initialization. Pointed out by clang with
drivers/staging/wfx/data_tx.c:34:19: warning: variable 'band' is uninitialized
when used here [-Wuninitialized]
if (rate->idx >= band->n_bitrates) {
Fixes: 868fd970e187 ("staging: wfx: improve robustness of wfx_get_hw_rate()")
Signed-off-by: Arnd Bergmann
---
drivers/staging/wfx/data_tx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 41f6a604a697..36b36ef39d05 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -31,13 +31,13 @@ static int wfx_get_hw_rate(struct wfx_dev *wdev,
}
return rate->idx + 14;
}
+ // WFx only support 2GHz, else band information should be retrieved
+ // from ieee80211_tx_info
+ band = wdev->hw->wiphy->bands[NL80211_BAND_2GHZ];
if (rate->idx >= band->n_bitrates) {
WARN(1, "wrong rate->idx value: %d", rate->idx);
return -1;
}
- // WFx only support 2GHz, else band information should be retrieved
- // from ieee80211_tx_info
- band = wdev->hw->wiphy->bands[NL80211_BAND_2GHZ];
return band->bitrates[rate->idx].hw_value;
}
--
2.27.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC] wimax: move out to staging
From: Arnd Bergmann
There are no known users of this driver as of October 2020, and it will
be removed unless someone turns out to still need it in future releases.
According to https://en.wikipedia.org/wiki/List_of_WiMAX_networks, there
have been many public wimax networks, but it appears that these entries
are all stale, after everyone has migrated to LTE or discontinued their
service altogether.
NetworkManager appears to have dropped userspace support in 2015
https://bugzilla.gnome.org/show_bug.cgi?id=747846, the
www.linuxwimax.org
site had already shut down earlier.
WiMax is apparently still being deployed on airport campus networks
("AeroMACS"), but in a frequency band that was not supported by the old
Intel 2400m (used in Sandy Bridge laptops and earlier), which is the
only driver using the kernel's wimax stack.
Move all files into drivers/staging/wimax, including the uapi header
files and documentation, to make it easier to remove it when it gets
to that. Only minimal changes are made to the source files, in order
to make it possible to port patches across the move.
Also remove the MAINTAINERS entry that refers to a broken mailing
list and website.
Suggested-by: Inaky Perez-Gonzalez
Signed-off-by: Arnd Bergmann
---
Documentation/admin-guide/index.rst | 1 -
Documentation/networking/kapi.rst | 21 --
.../translations/zh_CN/admin-guide/index.rst | 1 -
MAINTAINERS | 22 ---
drivers/net/Kconfig | 2 --
drivers/net/wimax/Kconfig | 18 ---
drivers/net/wimax/Makefile| 2 --
drivers/staging/Kconfig | 2 ++
drivers/staging/Makefile | 1 +
.../staging/wimax/Documentation}/i2400m.rst | 0
.../staging/wimax/Documentation}/index.rst| 0
.../staging/wimax/Documentation}/wimax.rst| 0
{net => drivers/staging}/wimax/Kconfig| 6 +
{net => drivers/staging}/wimax/Makefile | 2 ++
drivers/staging/wimax/TODO| 16 ++
{net => drivers/staging}/wimax/debug-levels.h | 2 +-
{net => drivers/staging}/wimax/debugfs.c | 2 +-
drivers/{net => staging}/wimax/i2400m/Kconfig | 0
.../{net => staging}/wimax/i2400m/Makefile| 0
.../{net => staging}/wimax/i2400m/control.c | 2 +-
.../wimax/i2400m/debug-levels.h | 2 +-
.../{net => staging}/wimax/i2400m/debugfs.c | 0
.../{net => staging}/wimax/i2400m/driver.c| 2 +-
drivers/{net => staging}/wimax/i2400m/fw.c| 0
.../wimax/i2400m/i2400m-usb.h | 0
.../{net => staging}/wimax/i2400m/i2400m.h| 4 ++--
.../staging/wimax/i2400m/linux-wimax-i2400m.h | 0
.../{net => staging}/wimax/i2400m/netdev.c| 0
.../{net => staging}/wimax/i2400m/op-rfkill.c | 2 +-
drivers/{net => staging}/wimax/i2400m/rx.c| 0
drivers/{net => staging}/wimax/i2400m/sysfs.c | 0
drivers/{net => staging}/wimax/i2400m/tx.c| 0
.../wimax/i2400m/usb-debug-levels.h | 2 +-
.../{net => staging}/wimax/i2400m/usb-fw.c| 0
.../{net => staging}/wimax/i2400m/usb-notif.c | 0
.../{net => staging}/wimax/i2400m/usb-rx.c| 0
.../{net => staging}/wimax/i2400m/usb-tx.c| 0
drivers/{net => staging}/wimax/i2400m/usb.c | 2 +-
{net => drivers/staging}/wimax/id-table.c | 2 +-
.../staging/wimax/linux-wimax-debug.h | 2 +-
.../staging/wimax/linux-wimax.h | 0
.../staging/wimax/net-wimax.h | 2 +-
{net => drivers/staging}/wimax/op-msg.c | 2 +-
{net => drivers/staging}/wimax/op-reset.c | 4 ++--
{net => drivers/staging}/wimax/op-rfkill.c| 4 ++--
{net => drivers/staging}/wimax/op-state-get.c | 4 ++--
{net => drivers/staging}/wimax/stack.c| 2 +-
.../staging}/wimax/wimax-internal.h | 2 +-
net/Kconfig | 2 --
net/Makefile | 1 -
50 files changed, 49 insertions(+), 92 deletions(-)
delete mode 100644 drivers/net/wimax/Kconfig
delete mode 100644 drivers/net/wimax/Makefile
rename {Documentation/admin-guide/wimax =>
drivers/staging/wimax/Documentation}/i2400m.rst (100%)
rename {Documentation/admin-guide/wimax =>
drivers/staging/wimax/Documentation}/index.rst (100%)
rename {Documentation/admin-guide/wimax =>
drivers/staging/wimax/Documentation}/wimax.rst (100%)
rename {net => drivers/staging}/wimax/Kconfig (94%)
rename {net => drivers/staging}/wimax/Makefile (83%)
create mode 100644 drivers/staging/wimax/TODO
rename {net => drivers/staging}/wimax/debug-levels.h (96%)
rename {net => drivers/staging}/wimax/debugfs.c (97%)
rename drivers/{net => staging}/wimax/i2400m/Kconfig (100%)
rename drivers/{net => staging}/wimax/i2400m/Makefile (100%)
rename drivers/{net =>
Re: [RFC] wimax: move out to staging
On Wed, Oct 28, 2020 at 11:34 AM Dan Carpenter wrote: > On Tue, Oct 27, 2020 at 10:20:13PM +0100, Arnd Bergmann wrote: > > From: Arnd Bergmann > > > > There are no known users of this driver as of October 2020, and it will > > be removed unless someone turns out to still need it in future releases. > > > > According to https://en.wikipedia.org/wiki/List_of_WiMAX_networks, there > > have been many public wimax networks, but it appears that these entries > > are all stale, after everyone has migrated to LTE or discontinued their > > service altogether. > > Wimax is still pretty common in Africa. But you have to buy an outdoor > antenae with all the software on it and an ethernet cable into your > house. Ah, good to know, thanks for the information. I'll include that when I resend the patch, which I have to do anyway to avoid a small regression. I did a look at a couple of African ISPs that seemed to all have discontinued service, but I suppose I should have looked more carefully. > I don't know what software the antennaes are using. Probably > Linux but with an out of tree kernel module is my guess. Right, it seems very unlikely that they would be using the old Intel drivers, and it's also unlikely that they are updating those boxes to new kernels. I found a firmware image for Huawei BM623m, which runs a proprietary kernel module for the wimax stack on an MT7108 (arm926) phone chip running a linux-2.6.26.8-rt16 kernel. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH, net -> staging, v2] wimax: move out to staging
From: Arnd Bergmann
There are no known users of this driver as of October 2020, and it will
be removed unless someone turns out to still need it in future releases.
According to https://en.wikipedia.org/wiki/List_of_WiMAX_networks, there
have been many public wimax networks, but it appears that many of these
have migrated to LTE or discontinued their service altogether.
As most PCs and phones lack WiMAX hardware support, the remaining
networks tend to use standalone routers. These almost certainly
run Linux, but not a modern kernel or the mainline wimax driver stack.
NetworkManager appears to have dropped userspace support in 2015
https://bugzilla.gnome.org/show_bug.cgi?id=747846, the
www.linuxwimax.org
site had already shut down earlier.
WiMax is apparently still being deployed on airport campus networks
("AeroMACS"), but in a frequency band that was not supported by the old
Intel 2400m (used in Sandy Bridge laptops and earlier), which is the
only driver using the kernel's wimax stack.
Move all files into drivers/staging/wimax, including the uapi header
files and documentation, to make it easier to remove it when it gets
to that. Only minimal changes are made to the source files, in order
to make it possible to port patches across the move.
Also remove the MAINTAINERS entry that refers to a broken mailing
list and website.
Suggested-by: Inaky Perez-Gonzalez
Signed-off-by: Arnd Bergmann
---
changes in v2:
- fix a build regression
- add more information about remaining networks (Dan Carpenter)_
For v1, Greg said he'd appply the patch when he gets an Ack
from the maintainers.
Inaky, Johannes, Jakub: are you happy with this version?
---
Documentation/admin-guide/index.rst | 1 -
Documentation/networking/kapi.rst | 21 --
.../translations/zh_CN/admin-guide/index.rst | 1 -
MAINTAINERS | 22 ---
drivers/net/Kconfig | 2 --
drivers/net/Makefile | 1 -
drivers/net/wimax/Kconfig | 18 ---
drivers/net/wimax/Makefile| 2 --
drivers/staging/Kconfig | 2 ++
drivers/staging/Makefile | 1 +
.../staging/wimax/Documentation}/i2400m.rst | 0
.../staging/wimax/Documentation}/index.rst| 0
.../staging/wimax/Documentation}/wimax.rst| 0
{net => drivers/staging}/wimax/Kconfig| 6 +
{net => drivers/staging}/wimax/Makefile | 2 ++
drivers/staging/wimax/TODO| 18 +++
{net => drivers/staging}/wimax/debug-levels.h | 2 +-
{net => drivers/staging}/wimax/debugfs.c | 2 +-
drivers/{net => staging}/wimax/i2400m/Kconfig | 0
.../{net => staging}/wimax/i2400m/Makefile| 0
.../{net => staging}/wimax/i2400m/control.c | 2 +-
.../wimax/i2400m/debug-levels.h | 2 +-
.../{net => staging}/wimax/i2400m/debugfs.c | 0
.../{net => staging}/wimax/i2400m/driver.c| 2 +-
drivers/{net => staging}/wimax/i2400m/fw.c| 0
.../wimax/i2400m/i2400m-usb.h | 0
.../{net => staging}/wimax/i2400m/i2400m.h| 4 ++--
.../staging/wimax/i2400m/linux-wimax-i2400m.h | 0
.../{net => staging}/wimax/i2400m/netdev.c| 0
.../{net => staging}/wimax/i2400m/op-rfkill.c | 2 +-
drivers/{net => staging}/wimax/i2400m/rx.c| 0
drivers/{net => staging}/wimax/i2400m/sysfs.c | 0
drivers/{net => staging}/wimax/i2400m/tx.c| 0
.../wimax/i2400m/usb-debug-levels.h | 2 +-
.../{net => staging}/wimax/i2400m/usb-fw.c| 0
.../{net => staging}/wimax/i2400m/usb-notif.c | 0
.../{net => staging}/wimax/i2400m/usb-rx.c| 0
.../{net => staging}/wimax/i2400m/usb-tx.c| 0
drivers/{net => staging}/wimax/i2400m/usb.c | 2 +-
{net => drivers/staging}/wimax/id-table.c | 2 +-
.../staging/wimax/linux-wimax-debug.h | 2 +-
.../staging/wimax/linux-wimax.h | 0
.../staging/wimax/net-wimax.h | 2 +-
{net => drivers/staging}/wimax/op-msg.c | 2 +-
{net => drivers/staging}/wimax/op-reset.c | 4 ++--
{net => drivers/staging}/wimax/op-rfkill.c| 4 ++--
{net => drivers/staging}/wimax/op-state-get.c | 4 ++--
{net => drivers/staging}/wimax/stack.c| 2 +-
.../staging}/wimax/wimax-internal.h | 2 +-
net/Kconfig | 2 --
net/Makefile | 1 -
51 files changed, 51 insertions(+), 93 deletions(-)
delete mode 100644 drivers/net/wimax/Kconfig
delete mode 100644 drivers/net/wimax/Makefile
rename {Documentation/admin-guide/wimax =>
drivers/staging/wimax/Documentation}/i2400m.rst (100%)
rename {Documentation/admin-guide/wimax =>
drivers/staging/wimax/Documentation}/index.rst (100%)
rename {Documentation/admin-guide/wimax =>
driv
Re: [RFC] wimax: move out to staging
n Thu, Oct 29, 2020 at 4:56 PM Jakub Kicinski wrote: > On Wed, 28 Oct 2020 06:56:28 +0100 Greg Kroah-Hartman wrote: > > On Tue, Oct 27, 2020 at 10:20:13PM +0100, Arnd Bergmann wrote: > > > > Is this ok for me to take through the staging tree? If so, I need an > > ack from the networking maintainers. > > > > If not, feel free to send it through the networking tree and add: > > > > Acked-by: Greg Kroah-Hartman > > Thinking about it now - we want this applied to -next, correct? > In that case it may be better if we take it. The code is pretty dead > but syzbot and the trivial fix crowd don't know it, so I may slip, > apply something and we'll have a conflict. I think git will deal with a merge between branches containing the move vs fix, so it should work either way. A downside of having the move in net-next would be that you'd get trivial fixes send to Greg, but him being unable to apply them to his tree because the code is elsewhere. If you think it helps, I could prepare a pull request with this one patch (and probably the bugfix I sent first that triggered it), and then you can both merge the branch into net-next as well as staging-next. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[GIT PULL, staging, net-next] wimax: move to staging
The following changes since commit 3650b228f83adda7e5ee532e2b90429c03f7b9ec:
Linux 5.10-rc1 (2020-10-25 15:14:11 -0700)
are available in the Git repository at:
git://git.kernel.org:/pub/scm/linux/kernel/git/arnd/playground.git
tags/wimax-staging
for you to fetch changes up to f54ec58fee837ec847cb8b50593e81bfaa46107f:
wimax: move out to staging (2020-10-29 19:27:45 +0100)
wimax: move to staging
After I sent a fix for what appeared to be a harmless warning in
the wimax user interface code, the conclusion was that the whole
thing has most likely not been used in a very long time, and the
user interface possibly been broken since b61a5eea5904 ("wimax: use
genl_register_family_with_ops()").
Using a shared branch between net-next and staging should help
coordinate patches getting submitted against it.
----
Arnd Bergmann (2):
wimax: fix duplicate initializer warning
wimax: move out to staging
Documentation/admin-guide/index.rst| 1 -
Documentation/networking/kapi.rst | 21
.../translations/zh_CN/admin-guide/index.rst | 1 -
MAINTAINERS| 22
drivers/net/Kconfig| 2 --
drivers/net/Makefile | 1 -
drivers/net/wimax/Kconfig | 18 --
drivers/net/wimax/Makefile | 2 --
drivers/staging/Kconfig| 2 ++
drivers/staging/Makefile | 1 +
.../staging/wimax/Documentation}/i2400m.rst| 0
.../staging/wimax/Documentation}/index.rst | 0
.../staging/wimax/Documentation}/wimax.rst | 0
{net => drivers/staging}/wimax/Kconfig | 6 +
{net => drivers/staging}/wimax/Makefile| 2 ++
drivers/staging/wimax/TODO | 18 ++
{net => drivers/staging}/wimax/debug-levels.h | 2 +-
{net => drivers/staging}/wimax/debugfs.c | 2 +-
drivers/{net => staging}/wimax/i2400m/Kconfig | 0
drivers/{net => staging}/wimax/i2400m/Makefile | 0
drivers/{net => staging}/wimax/i2400m/control.c| 2 +-
.../{net => staging}/wimax/i2400m/debug-levels.h | 2 +-
drivers/{net => staging}/wimax/i2400m/debugfs.c| 0
drivers/{net => staging}/wimax/i2400m/driver.c | 2 +-
drivers/{net => staging}/wimax/i2400m/fw.c | 0
drivers/{net => staging}/wimax/i2400m/i2400m-usb.h | 0
drivers/{net => staging}/wimax/i2400m/i2400m.h | 4 +--
.../staging/wimax/i2400m/linux-wimax-i2400m.h | 0
drivers/{net => staging}/wimax/i2400m/netdev.c | 0
drivers/{net => staging}/wimax/i2400m/op-rfkill.c | 2 +-
drivers/{net => staging}/wimax/i2400m/rx.c | 0
drivers/{net => staging}/wimax/i2400m/sysfs.c | 0
drivers/{net => staging}/wimax/i2400m/tx.c | 0
.../wimax/i2400m/usb-debug-levels.h| 2 +-
drivers/{net => staging}/wimax/i2400m/usb-fw.c | 0
drivers/{net => staging}/wimax/i2400m/usb-notif.c | 0
drivers/{net => staging}/wimax/i2400m/usb-rx.c | 0
drivers/{net => staging}/wimax/i2400m/usb-tx.c | 0
drivers/{net => staging}/wimax/i2400m/usb.c| 2 +-
{net => drivers/staging}/wimax/id-table.c | 2 +-
.../staging/wimax/linux-wimax-debug.h | 2 +-
.../wimax.h => drivers/staging/wimax/linux-wimax.h | 0
.../wimax.h => drivers/staging/wimax/net-wimax.h | 2 +-
{net => drivers/staging}/wimax/op-msg.c| 2 +-
{net => drivers/staging}/wimax/op-reset.c | 4 +--
{net => drivers/staging}/wimax/op-rfkill.c | 4 +--
{net => drivers/staging}/wimax/op-state-get.c | 4 +--
{net => drivers/staging}/wimax/stack.c | 29 ++
{net => drivers/staging}/wimax/wimax-internal.h| 2 +-
net/Kconfig| 2 --
net/Makefile | 1 -
51 files changed, 68 insertions(+), 103 deletions(-)
delete mode 100644 drivers/net/wimax/Kconfig
delete mode 100644 drivers/net/wimax/Makefile
rename {Documentation/admin-guide/wimax =>
drivers/staging/wimax/Documentation}/i2400m.rst (100%)
rename {Documentation/admin-guide/wimax =>
drivers/staging/wimax/Documentation}/index.rst (100%)
rename {Documentation/admin-guide/wimax =>
drivers/staging/wimax/Documentation}/wimax.rst (100%)
rename {net => drivers/staging}/wimax/Kconfig (94%)
rename {net => drivers/staging}/wimax/Makefile (83%)
create mode 100644 drivers/staging/wimax/TODO
rename {net => drivers/staging}/wimax/debug-levels.h (96%)
rename {net => drivers/staging}/wimax/debugfs.c (97%)
rename drivers/{net =>
Re: [PATCH v2] staging: trivial: hikey9xx: fix be32<->u32 casting warnings
On Thu, Nov 19, 2020 at 1:31 PM Juan Antonio Aldea-Armenteros
wrote:
>
> This patch fixes the following warnings reported by sparse, by adding
> missing __force annotations.
>
> drivers/staging/hikey9xx/hisi-spmi-controller.c:164:24: warning: cast to
> restricted __be32
> drivers/staging/hikey9xx/hisi-spmi-controller.c:164:24: warning: cast to
> restricted __be32
> drivers/staging/hikey9xx/hisi-spmi-controller.c:164:24: warning: cast to
> restricted __be32
> drivers/staging/hikey9xx/hisi-spmi-controller.c:164:24: warning: cast to
> restricted __be32
> drivers/staging/hikey9xx/hisi-spmi-controller.c:164:24: warning: cast to
> restricted __be32
> drivers/staging/hikey9xx/hisi-spmi-controller.c:164:24: warning: cast to
> restricted __be32
>
> drivers/staging/hikey9xx/hisi-spmi-controller.c:239:25: warning: cast from
> restricted __be32
>
> Rationale for #164:
> data is declared as u32, and it is read and then converted by means of
> be32_to_cpu(). Said function expects a __be32 but data is u32, therefore
> there's a type missmatch here.
>
> Rationale for #239:
> Is the dual of #164. This time data going to be written so it
> needs to be converted from cpu to __be32, but writel() expects u32 and the
> output of cpu_to_be32 returns a __be32.
Both of the casts look very suspicious, I'd leave these in unless
someone can confirm what the actual desired behavior is.
> SPMI_SLAVE_OFFSET * slave_id +
> SPMI_APB_SPMI_RDATA0_BASE_ADDR +
> i * SPMI_PER_DATAREG_BYTE);
> - data = be32_to_cpu((__be32)data);
> + data = be32_to_cpu((__be32 __force)data);
> if ((bc - i * SPMI_PER_DATAREG_BYTE) >> 2) {
> memcpy(buf, &data, sizeof(data));
> buf += sizeof(data);
The data comes from a readl(), which contains an endian conversion
on architectures that need it, such as when running the board
in big-endian arm64 mode. Having a second endian-conversion
on little-endian architectures means that the data is always swapped
when it gets written to the register.
In the original code before Mauro's commit 8788a30c12c7 ("staging:
spmi: hisi-spmi-controller: use le32 macros where needed"), the data
was byteswapped, and then written into the fifo register, which
produced no warning but would do a double-swap on a big-endian
kernel, and change the behavior from what it was before.
My guess is that Mauro inadvertently fixed this driver for big-endian
mode, without noticing that it was broken to start with, and that
he did not actually try it with CONFIG_CPU_BIG_ENDIAN.
I think the best way would be to go back to to using swab32p()
(not the open-coded version) and then use writesl() or
iowrite32_rep() with count=1 to write the byteswapped FIFO
register without swapping it again.
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: bcm2835: fix vchiq_mmal dependencies
From: Arnd Bergmann
When the MMAL code is built-in but the vchiq core config is
set to =m, the mmal code never gets built, which in turn can
lead to link errors:
ERROR: modpost: "vchiq_mmal_port_set_format"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_port_disable"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_port_parameter_set"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_component_finalise"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_port_connect_tunnel"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_component_enable"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_finalise"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_component_init"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_component_disable"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "mmal_vchi_buffer_init"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_port_enable"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_version"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_submit_buffer"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_init"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "mmal_vchi_buffer_cleanup"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
ERROR: modpost: "vchiq_mmal_port_parameter_get"
[drivers/staging/vc04_services/bcm2835-camera/bcm2835-v4l2.ko] undefined!
Change the Kconfig to depend on BCM2835_VCHIQ like the other drivers,
and remove the now redundant dependencies.
Fixes: b18ee53ad297 ("staging: bcm2835: Break MMAL support out from camera")
Signed-off-by: Arnd Bergmann
---
drivers/staging/vc04_services/vchiq-mmal/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vc04_services/vchiq-mmal/Kconfig
b/drivers/staging/vc04_services/vchiq-mmal/Kconfig
index 500c0d12e4ff..c99525a0bb45 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/Kconfig
+++ b/drivers/staging/vc04_services/vchiq-mmal/Kconfig
@@ -1,6 +1,6 @@
config BCM2835_VCHIQ_MMAL
tristate "BCM2835 MMAL VCHIQ service"
- depends on (ARCH_BCM2835 || COMPILE_TEST)
+ depends on BCM2835_VCHIQ
help
Enables the MMAL API over VCHIQ interface as used for the
majority of the multimedia services on VideoCore.
--
2.27.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: bcm2835: fix vchiq_mmal dependencies
On Fri, Dec 4, 2020 at 11:44 AM Jacopo Mondi wrote:
>
> Hi Arnd,
>
> On Thu, Dec 03, 2020 at 11:38:30PM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann
> >
> > When the MMAL code is built-in but the vchiq core config is
> > set to =m, the mmal code never gets built, which in turn can
> > lead to link errors:
>
> My bad, I repetedly ignored the error report received from the 'kernel
> test robot' about this. Thanks for fixing.
>
> For my eduction, why would the vchiq-mmal code not get build if
> vchiq-core is set to M ? I mean, that configuration is indeed wrong,
> as vchiq-mmal uses symbols from vchiq-core and I would expect that to
> fail when building the kernel image, not have the other modules (as
> bcm2835-camera) fail as a consequence when building modules.
drivers/staging/Makefile has this line:
obj-$(CONFIG_BCM2835_VCHIQ) += vc04_services/
when CONFIG_BCM2835_VCHIQ=m, the kbuild infrastructure
only enters the subdirectory while building modules, but a built-in
mmal driver is not a loadable module, so it does not get built
at that time. When compiling the built-in code, the subdirectory is
not entered.
> > Fixes: b18ee53ad297 ("staging: bcm2835: Break MMAL support out from camera")
> > Signed-off-by: Arnd Bergmann
>
> Acked-by: Jacopo Mondi
>
> If you noticed this from the same error notification I recevied it
> might be fair to report:
> Reported-by: kernel test robot
I had not seen that report but found it during my own testing,
thanks for adding.
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl819x: select CONFIG_CRC32
From: Arnd Bergmann Without crc32 support, the drivers fail to link: ERROR: modpost: "crc32_le" [drivers/staging/rtl8192e/rtllib_crypt_wep.ko] undefined! ERROR: modpost: "crc32_le" [drivers/staging/rtl8192e/rtllib_crypt_tkip.ko] undefined! ERROR: modpost: "crc32_le" [drivers/staging/rtl8192u/r8192u_usb.ko] undefined! Signed-off-by: Arnd Bergmann --- drivers/staging/rtl8192e/Kconfig | 1 + drivers/staging/rtl8192u/Kconfig | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/staging/rtl8192e/Kconfig b/drivers/staging/rtl8192e/Kconfig index 03fcc23516fd..963a2ffbc1fb 100644 --- a/drivers/staging/rtl8192e/Kconfig +++ b/drivers/staging/rtl8192e/Kconfig @@ -3,6 +3,7 @@ config RTLLIB tristate "Support for rtllib wireless devices" depends on WLAN && m select LIB80211 + select CRC32 help If you have a wireless card that uses rtllib, say Y. Currently the only card is the rtl8192e. diff --git a/drivers/staging/rtl8192u/Kconfig b/drivers/staging/rtl8192u/Kconfig index ef883d462d3d..f3b112a058ca 100644 --- a/drivers/staging/rtl8192u/Kconfig +++ b/drivers/staging/rtl8192u/Kconfig @@ -5,6 +5,7 @@ config RTL8192U depends on m select WIRELESS_EXT select WEXT_PRIV + select CRC32 select CRYPTO select CRYPTO_AES select CRYPTO_CCM -- 2.29.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: greybus: fix stack size warning with UBSAN
From: Arnd Bergmann
clang warns about excessive stack usage in this driver when
UBSAN is enabled:
drivers/staging/greybus/audio_topology.c:977:12: error: stack frame size of
1836 bytes in function 'gbaudio_tplg_create_widget'
[-Werror,-Wframe-larger-than=]
Rework this code to no longer use compound literals for
initializing the structure in each case, but instead keep
the common bits in a preallocated constant array and copy
them as needed.
Signed-off-by: Arnd Bergmann
---
drivers/staging/greybus/audio_topology.c | 106 ++-
1 file changed, 47 insertions(+), 59 deletions(-)
diff --git a/drivers/staging/greybus/audio_topology.c
b/drivers/staging/greybus/audio_topology.c
index 96b8b29fe899..c03873915c20 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -974,6 +974,44 @@ static int gbaudio_widget_event(struct snd_soc_dapm_widget
*w,
return ret;
}
+static const struct snd_soc_dapm_widget gbaudio_widgets[] = {
+ [snd_soc_dapm_spk] = SND_SOC_DAPM_SPK("spk", gbcodec_event_spk),
+ [snd_soc_dapm_hp] = SND_SOC_DAPM_HP("hp", gbcodec_event_hp),
+ [snd_soc_dapm_mic] = SND_SOC_DAPM_MIC("mic",
gbcodec_event_int_mic),
+ [snd_soc_dapm_output] = SND_SOC_DAPM_OUTPUT("output"),
+ [snd_soc_dapm_input]= SND_SOC_DAPM_INPUT("input"),
+ [snd_soc_dapm_switch] = SND_SOC_DAPM_SWITCH_E("switch", SND_SOC_NOPM,
+ 0, 0, NULL,
+ gbaudio_widget_event,
+ SND_SOC_DAPM_PRE_PMU |
+ SND_SOC_DAPM_POST_PMD),
+ [snd_soc_dapm_pga] = SND_SOC_DAPM_PGA_E("pga", SND_SOC_NOPM,
+ 0, 0, NULL, 0,
+ gbaudio_widget_event,
+ SND_SOC_DAPM_PRE_PMU |
+ SND_SOC_DAPM_POST_PMD),
+ [snd_soc_dapm_mixer]= SND_SOC_DAPM_MIXER_E("mixer", SND_SOC_NOPM,
+ 0, 0, NULL, 0,
+ gbaudio_widget_event,
+ SND_SOC_DAPM_PRE_PMU |
+ SND_SOC_DAPM_POST_PMD),
+ [snd_soc_dapm_mux] = SND_SOC_DAPM_MUX_E("mux", SND_SOC_NOPM,
+ 0, 0, NULL,
+ gbaudio_widget_event,
+ SND_SOC_DAPM_PRE_PMU |
+ SND_SOC_DAPM_POST_PMD),
+ [snd_soc_dapm_aif_in] = SND_SOC_DAPM_AIF_IN_E("aif_in", NULL, 0,
+ SND_SOC_NOPM, 0, 0,
+ gbaudio_widget_event,
+ SND_SOC_DAPM_PRE_PMU |
+ SND_SOC_DAPM_POST_PMD),
+ [snd_soc_dapm_aif_out] = SND_SOC_DAPM_AIF_OUT_E("aif_out", NULL, 0,
+ SND_SOC_NOPM, 0, 0,
+ gbaudio_widget_event,
+ SND_SOC_DAPM_PRE_PMU |
+ SND_SOC_DAPM_POST_PMD),
+};
+
static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
struct snd_soc_dapm_widget *dw,
struct gb_audio_widget *w, int *w_size)
@@ -1050,78 +1088,28 @@ static int gbaudio_tplg_create_widget(struct
gbaudio_module_info *module,
strlcpy(temp_name, w->name, NAME_SIZE);
snprintf(w->name, NAME_SIZE, "GB %d %s", module->dev_id, temp_name);
+ if (w->type > ARRAY_SIZE(gbaudio_widgets)) {
+ ret = -EINVAL;
+ goto error;
+ }
+ *dw = gbaudio_widgets[w->type];
+ dw->name = w->name;
+
switch (w->type) {
case snd_soc_dapm_spk:
- *dw = (struct snd_soc_dapm_widget)
- SND_SOC_DAPM_SPK(w->name, gbcodec_event_spk);
module->op_devices |= GBAUDIO_DEVICE_OUT_SPEAKER;
break;
case snd_soc_dapm_hp:
- *dw = (struct snd_soc_dapm_widget)
- SND_SOC_DAPM_HP(w->name, gbcodec_event_hp);
module->op_devices |= (GBAUDIO_DEVICE_OUT_WIRED_HEADSET
- | GBAUDIO_DEVICE_OUT_WIRED_HEADPHONE);
+ | GBAUDIO_DEVICE_OUT_WIRED_HEADPHONE),
module->ip_devices |= GBAUDIO_DEVICE_IN_WIRED_HEADSET;
break;
case snd_soc_dapm_mic:
- *dw = (struct snd_soc_dapm_widget)
- SND_SOC_DAPM_MIC(w->
[PATCH] staging: vchiq: fix uninitialized variable copy
From: Arnd Bergmann
Smatch found a local variable that can get copied to another
local variable without an initializion in the error case:
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1056
vchiq_get_user_ptr() error: uninitialized symbol 'ptr'.
This seems harmless, as the function should normally get inlined, with
the output directly written or not. In any case, the uninitialized data
is never used after get_user() fails.
As Dan mentions, it could still trigger an UBSAN runtime error, and it
is of course a bad idea to copy uninitialized variables, so just
bail out early.
Reported-by: kernel test robot
Reported-by: Dan Carpenter
Signed-off-by: Arnd Bergmann
---
.../vc04_services/interface/vchiq_arm/vchiq_arm.c| 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index f500a7043805..63a0045ef9c5 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1057,14 +1057,21 @@ static inline int vchiq_get_user_ptr(void __user **buf,
void __user *ubuf, int i
compat_uptr_t ptr32;
compat_uptr_t __user *uptr = ubuf;
ret = get_user(ptr32, uptr + index);
+ if (ret)
+ return ret;
+
*buf = compat_ptr(ptr32);
} else {
uintptr_t ptr, __user *uptr = ubuf;
ret = get_user(ptr, uptr + index);
+
+ if (ret)
+ return ret;
+
*buf = (void __user *)ptr;
}
- return ret;
+ return 0;
}
struct vchiq_completion_data32 {
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vchiq: delete obselete comment
On Tue, Jan 5, 2021 at 2:19 PM Dan Carpenter wrote:
>
> This comment describes a security problem which was fixed in commit
> 1c954540c0eb ("staging: vchiq: avoid mixing kernel and user pointers").
> The bug is fixed now so the FIXME can be removed.
>
> Signed-off-by: Dan Carpenter
Reviewed-by: Arnd Bergmann
There is still another sparse warning for a remaining __user address
space mismatch in the driver, but this one seems to be fixed as you
say. Thanks for the fix!
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: vchiq: Fix bulk userdata handling
On Tue, Jan 5, 2021 at 12:53 PM Phil Elwell wrote: > On Tue, 5 Jan 2021 at 11:04, Dan Carpenter wrote: > > > > Mixing __user pointers and regular pointers is dangerous and has lead to > > security problems in this driver in the past. But also mixing mixing > > tokens with pointers just makes the code hard to read. Instead of > > undoing Arnd's work where he split the user space and kernel pointers > > apart we should go ahead and spit it up even more. At least add a giant > > FIXME comment and an item in the TODO list so we don't forget to do this > > before removing the code from staging. > > Those all sound like valid comments to have made against the original > patch, but that seems to have received little attention. > > I'll just leave this here - perhaps Arnd has the patience to finish the job. I don't really have an interest in this driver. I did a larger cleanup in order to kill off copy_in_user() from the kernel, and then cleaned it up some more for good measure, but I would hope someone else can finish the address space mismatch. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/3] staging: vchiq: Fix bulk transfers on 64-bit builds
On Tue, Jan 5, 2021 at 5:20 PM Phil Elwell wrote:
>
> The recent change to the bulk transfer compat function missed the fact
> the relevant ioctl command is VCHIQ_IOC_QUEUE_BULK_TRANSMIT32, not
> VCHIQ_IOC_QUEUE_BULK_TRANSMIT, as any attempt to send a bulk block
> to the VPU would have shown.
>
> Fixes: a4367cd2b231 ("staging: vchiq: convert compat bulk transfer")
>
> Signed-off-by: Phil Elwell
Acked-by: Arnd Bergmann
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: atomisp: fix Wvisiblity warning
From: Arnd Bergmann
Some randconfig builds include ia_css_firmware.h without first
including linux/device.h:
In file included from atomisp/pci/mmu/sh_mmu_mrfld.c:23:
In file included from atomisp/pci/atomisp_compat.h:22:
In file included from atomisp/pci/atomisp_compat_css20.h:24:
In file included from atomisp/pci/ia_css.h:28:
In file included from atomisp/pci/ia_css_control.h:25:
drivers/staging/media/atomisp//pci/ia_css_firmware.h:52:29: error: declaration
of 'struct device' will not be visible outside of this function
[-Werror,-Wvisibility]
Add a forward declaration to avoid the warning.
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/atomisp/pci/ia_css_firmware.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/media/atomisp/pci/ia_css_firmware.h
b/drivers/staging/media/atomisp/pci/ia_css_firmware.h
index edab72256494..38f0408947d1 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_firmware.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_firmware.h
@@ -30,6 +30,8 @@ struct ia_css_fw {
unsigned int bytes; /** length in bytes of firmware data */
};
+struct device;
+
/* @brief Loads the firmware
* @param[in] env Environment, provides functions to access the
* environment in which the CSS code runs. This is
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3] staging: vchiq: fix completion routines & semaphores
On Wed, Dec 12, 2018 at 7:54 PM Nicolas Saenz Julienne wrote: > > As pointed out by Arnd Bergman[1] there where some issues on how I > converted semaphores to completion routines. I wasn't aware of a > macro override happening in vchiq_killable.h, which was changing > down_interruptible()'s meaning. > > This patch changes all completions so they use the proper APIs and gets > rid of vchiq_killable.h. I took into account Arnd's commit to avoid > merge conflicts. > Reviewed-by: Arnd Bergmann Thanks for the follow-up! Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/7] staging: olpc_dcon: add a missing dependency
From: Lubomir Rintel Commit 33f49571d75024b1044cd02689ad2bdb4924cc80 upstream. WARNING: unmet direct dependencies detected for BACKLIGHT_CLASS_DEVICE Depends on [n]: HAS_IOMEM [=y] && BACKLIGHT_LCD_SUPPORT [=n] Selected by [y]: - FB_OLPC_DCON [=y] && STAGING [=y] && X86 [=y] && OLPC [=y] && FB [=y] && I2C [=y] && (GPIO_CS5535 [=n] || GPIO_CS5535 [=n]=n) Signed-off-by: Lubomir Rintel Signed-off-by: Greg Kroah-Hartman --- drivers/staging/olpc_dcon/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/olpc_dcon/Kconfig b/drivers/staging/olpc_dcon/Kconfig index c91a56f77bcb..192cc8d0853f 100644 --- a/drivers/staging/olpc_dcon/Kconfig +++ b/drivers/staging/olpc_dcon/Kconfig @@ -2,6 +2,7 @@ config FB_OLPC_DCON tristate "One Laptop Per Child Display CONtroller support" depends on OLPC && FB depends on I2C + depends on BACKLIGHT_LCD_SUPPORT depends on (GPIO_CS5535 || GPIO_CS5535=n) select BACKLIGHT_CLASS_DEVICE help -- 2.20.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: axis-fifo: add CONFIG_OF dependency
When building without CONFIG_OF, the compiler loses track of the flow
control in axis_fifo_probe(), and thinks that many variables are used
without an initialization even though we actually leave the function
before the first use:
drivers/staging/axis-fifo/axis-fifo.c: In function 'axis_fifo_probe':
drivers/staging/axis-fifo/axis-fifo.c:900:5: error: 'rxd_tdata_width' may be
used uninitialized in this function [-Werror=maybe-uninitialized]
if (rxd_tdata_width != 32) {
^
drivers/staging/axis-fifo/axis-fifo.c:907:5: error: 'txd_tdata_width' may be
used uninitialized in this function [-Werror=maybe-uninitialized]
if (txd_tdata_width != 32) {
^
drivers/staging/axis-fifo/axis-fifo.c:914:5: error: 'has_tdest' may be used
uninitialized in this function [-Werror=maybe-uninitialized]
if (has_tdest) {
^
drivers/staging/axis-fifo/axis-fifo.c:919:5: error: 'has_tid' may be used
uninitialized in this function [-Werror=maybe-uninitialized]
When CONFIG_OF is set, this does not happen, and since the driver cannot
work without it, just add that option as a Kconfig dependency.
Signed-off-by: Arnd Bergmann
---
drivers/staging/axis-fifo/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/axis-fifo/Kconfig
b/drivers/staging/axis-fifo/Kconfig
index 687537203d9c..d9725888af6f 100644
--- a/drivers/staging/axis-fifo/Kconfig
+++ b/drivers/staging/axis-fifo/Kconfig
@@ -3,6 +3,7 @@
#
config XIL_AXIS_FIFO
tristate "Xilinx AXI-Stream FIFO IP core driver"
+ depends on OF
default n
help
This adds support for the Xilinx AXI-Stream
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: olpc_dcon_xo_1: add missing 'const' qualifier
gcc noticed a mismatch between the type qualifiers after a recent
cleanup:
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c: In function 'dcon_init_xo_1':
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c:48:26: error: initialization
discards 'const' qualifier from pointer target type
[-Werror=discarded-qualifiers]
Add the 'const' keyword that should have been there all along.
Fixes: 2159fb372929 ("staging: olpc_dcon: olpc_dcon_xo_1.c: Switch to the gpio
descriptor interface")
Signed-off-by: Arnd Bergmann
---
drivers/staging/olpc_dcon/olpc_dcon_xo_1.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/olpc_dcon/olpc_dcon_xo_1.c
b/drivers/staging/olpc_dcon/olpc_dcon_xo_1.c
index 80b8d4153414..a54286498a47 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon_xo_1.c
+++ b/drivers/staging/olpc_dcon/olpc_dcon_xo_1.c
@@ -45,7 +45,7 @@ static int dcon_init_xo_1(struct dcon_priv *dcon)
{
unsigned char lob;
int ret, i;
- struct dcon_gpio *pin = &gpios_asis[0];
+ const struct dcon_gpio *pin = &gpios_asis[0];
for (i = 0; i < ARRAY_SIZE(gpios_asis); i++) {
gpios[i] = devm_gpiod_get(&dcon->client->dev, pin[i].name,
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: staging/intel-ipu3: reduce kernel stack usage
The imgu_css_queue structure is too large to be put on the kernel
stack, as we can see in 32-bit builds:
drivers/staging/media/ipu3/ipu3-css.c: In function 'imgu_css_fmt_try':
drivers/staging/media/ipu3/ipu3-css.c:1863:1: error: the frame size of 1172
bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
By dynamically allocating this array, the stack usage goes down to an
acceptable 140 bytes for the same x86-32 configuration.
Fixes: f5f2e4273518 ("media: staging/intel-ipu3: Add css pipeline programming")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/ipu3/ipu3-css.c | 25 +++--
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/media/ipu3/ipu3-css.c
b/drivers/staging/media/ipu3/ipu3-css.c
index 15ab77e4b766..664c14b7a518 100644
--- a/drivers/staging/media/ipu3/ipu3-css.c
+++ b/drivers/staging/media/ipu3/ipu3-css.c
@@ -3,6 +3,7 @@
#include
#include
+#include
#include "ipu3-css.h"
#include "ipu3-css-fw.h"
@@ -1744,7 +1745,7 @@ int imgu_css_fmt_try(struct imgu_css *css,
struct v4l2_rect *const bds = &r[IPU3_CSS_RECT_BDS];
struct v4l2_rect *const env = &r[IPU3_CSS_RECT_ENVELOPE];
struct v4l2_rect *const gdc = &r[IPU3_CSS_RECT_GDC];
- struct imgu_css_queue q[IPU3_CSS_QUEUES];
+ struct imgu_css_queue *q = kcalloc(IPU3_CSS_QUEUES, sizeof(struct
imgu_css_queue), GFP_KERNEL);
struct v4l2_pix_format_mplane *const in =
&q[IPU3_CSS_QUEUE_IN].fmt.mpix;
struct v4l2_pix_format_mplane *const out =
@@ -1753,6 +1754,11 @@ int imgu_css_fmt_try(struct imgu_css *css,
&q[IPU3_CSS_QUEUE_VF].fmt.mpix;
int i, s, ret;
+ if (!q) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
/* Adjust all formats, get statistics buffer sizes and formats */
for (i = 0; i < IPU3_CSS_QUEUES; i++) {
if (fmts[i])
@@ -1766,7 +1772,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
IPU3_CSS_QUEUE_TO_FLAGS(i))) {
dev_notice(css->dev, "can not initialize queue %s\n",
qnames[i]);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
}
for (i = 0; i < IPU3_CSS_RECTS; i++) {
@@ -1788,7 +1795,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
if (!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_IN]) ||
!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_OUT])) {
dev_warn(css->dev, "required queues are disabled\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
if (!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_OUT])) {
@@ -1829,7 +1837,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
ret = imgu_css_find_binary(css, pipe, q, r);
if (ret < 0) {
dev_err(css->dev, "failed to find suitable binary\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
css->pipes[pipe].bindex = ret;
@@ -1843,7 +1852,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
IPU3_CSS_QUEUE_TO_FLAGS(i))) {
dev_err(css->dev,
"final resolution adjustment failed\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
*fmts[i] = q[i].fmt.mpix;
}
@@ -1859,7 +1869,10 @@ int imgu_css_fmt_try(struct imgu_css *css,
bds->width, bds->height, gdc->width, gdc->height,
out->width, out->height, vf->width, vf->height);
- return 0;
+ ret = 0;
+out:
+ kfree(q);
+ return ret;
}
int imgu_css_fmt_set(struct imgu_css *css,
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: staging/intel-ipu3-v4l: reduce kernel stack usage
The v4l2_pix_format_mplane structure is too large to be put on the kernel
stack, as we can see in 32-bit builds:
drivers/staging/media/ipu3/ipu3-v4l2.c: In function 'imgu_fmt':
drivers/staging/media/ipu3/ipu3-v4l2.c:753:1: error: the frame size of 1028
bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
By dynamically allocating this array, the stack usage goes down to an
acceptable 272 bytes for the same x86-32 configuration.
Fixes: a0ca1627b450 ("media: staging/intel-ipu3: Add v4l2 driver based on media
framework")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/ipu3/ipu3-v4l2.c | 40 --
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c
b/drivers/staging/media/ipu3/ipu3-v4l2.c
index 9c0352b193a7..c34b433539c4 100644
--- a/drivers/staging/media/ipu3/ipu3-v4l2.c
+++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
@@ -664,12 +664,11 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned
int pipe, int node,
struct v4l2_format *f, bool try)
{
struct device *dev = &imgu->pci_dev->dev;
- struct v4l2_pix_format_mplane try_fmts[IPU3_CSS_QUEUES];
struct v4l2_pix_format_mplane *fmts[IPU3_CSS_QUEUES] = { NULL };
struct v4l2_rect *rects[IPU3_CSS_RECTS] = { NULL };
struct v4l2_mbus_framefmt pad_fmt;
unsigned int i, css_q;
- int r;
+ int ret;
struct imgu_css_pipe *css_pipe = &imgu->css.pipes[pipe];
struct imgu_media_pipe *imgu_pipe = &imgu->imgu_pipe[pipe];
struct imgu_v4l2_subdev *imgu_sd = &imgu_pipe->imgu_sd;
@@ -698,9 +697,13 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int
pipe, int node,
continue;
if (try) {
- try_fmts[i] =
- imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp;
- fmts[i] = &try_fmts[i];
+ fmts[i] =
kmemdup(&imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp,
+ sizeof(struct v4l2_pix_format_mplane),
+ GFP_KERNEL);
+ if (!fmts[i]) {
+ ret = -ENOMEM;
+ goto out;
+ }
} else {
fmts[i] = &imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp;
}
@@ -730,26 +733,33 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned
int pipe, int node,
* before we return success from this function, so set it here.
*/
css_q = imgu_node_to_queue(node);
- if (fmts[css_q])
- *fmts[css_q] = f->fmt.pix_mp;
- else
- return -EINVAL;
+ if (!fmts[css_q]) {
+ ret = -EINVAL;
+ goto out;
+ }
+ *fmts[css_q] = f->fmt.pix_mp;
if (try)
- r = imgu_css_fmt_try(&imgu->css, fmts, rects, pipe);
+ ret = imgu_css_fmt_try(&imgu->css, fmts, rects, pipe);
else
- r = imgu_css_fmt_set(&imgu->css, fmts, rects, pipe);
+ ret = imgu_css_fmt_set(&imgu->css, fmts, rects, pipe);
- /* r is the binary number in the firmware blob */
- if (r < 0)
- return r;
+ /* ret is the binary number in the firmware blob */
+ if (ret < 0)
+ goto out;
if (try)
f->fmt.pix_mp = *fmts[css_q];
else
f->fmt = imgu_pipe->nodes[node].vdev_fmt.fmt;
- return 0;
+out:
+ if (try) {
+ for (i = 0; i < IPU3_CSS_QUEUES; i++)
+ kfree(fmts[i]);
+ }
+
+ return ret;
}
static int imgu_try_fmt(struct file *file, void *fh, struct v4l2_format *f)
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: staging/intel-ipu3: mark PM function as __maybe_unused
The imgu_rpm_dummy_cb() looks like an API misuse that is explained
in the comment above it. Aside from that, it also causes a warning
when power management support is disabled:
drivers/staging/media/ipu3/ipu3.c:794:12: error: 'imgu_rpm_dummy_cb' defined
but not used [-Werror=unused-function]
The warning is at least easy to fix by marking the function as
__maybe_unused.
Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device
driver")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/ipu3/ipu3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/ipu3/ipu3.c
b/drivers/staging/media/ipu3/ipu3.c
index d575ac78c8f0..d00d26264c37 100644
--- a/drivers/staging/media/ipu3/ipu3.c
+++ b/drivers/staging/media/ipu3/ipu3.c
@@ -791,7 +791,7 @@ static int __maybe_unused imgu_resume(struct device *dev)
* PCI rpm framework checks the existence of driver rpm callbacks.
* Place a dummy callback here to avoid rpm going into error state.
*/
-static int imgu_rpm_dummy_cb(struct device *dev)
+static __maybe_unused int imgu_rpm_dummy_cb(struct device *dev)
{
return 0;
}
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: staging: davinci_vpfe: disallow building with COMPILE_TEST
The driver should really call dm365_isif_setup_pinmux() through a callback,
but it runs platform specific code by itself, which never actually compiled:
/git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:2: error:
implicit declaration of function 'davinci_cfg_reg'
[-Werror,-Wimplicit-function-declaration]
davinci_cfg_reg(DM365_VIN_CAM_WEN);
^
/git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:2: error:
this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
/git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:18: error:
use of undeclared identifier 'DM365_VIN_CAM_WEN'
davinci_cfg_reg(DM365_VIN_CAM_WEN);
^
/git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2029:18: error:
use of undeclared identifier 'DM365_VIN_CAM_VD'
davinci_cfg_reg(DM365_VIN_CAM_VD);
^
/git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2030:18: error:
use of undeclared identifier 'DM365_VIN_CAM_HD'
davinci_cfg_reg(DM365_VIN_CAM_HD);
^
/git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2031:18: error:
use of undeclared identifier 'DM365_VIN_YIN4_7_EN'
davinci_cfg_reg(DM365_VIN_YIN4_7_EN);
^
/git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2032:18: error:
use of undeclared identifier 'DM365_VIN_YIN0_3_EN'
davinci_cfg_reg(DM365_VIN_YIN0_3_EN);
^
7 errors generated.
Fixes: 4907c73deefe ("media: staging: davinci_vpfe: allow building with
COMPILE_TEST")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/davinci_vpfe/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/davinci_vpfe/Kconfig
b/drivers/staging/media/davinci_vpfe/Kconfig
index aea449a8dbf8..84ac6b9e1767 100644
--- a/drivers/staging/media/davinci_vpfe/Kconfig
+++ b/drivers/staging/media/davinci_vpfe/Kconfig
@@ -1,7 +1,7 @@
config VIDEO_DM365_VPFE
tristate "DM365 VPFE Media Controller Capture Driver"
depends on VIDEO_V4L2
- depends on (ARCH_DAVINCI_DM365 && !VIDEO_DM365_ISIF) || COMPILE_TEST
+ depends on (ARCH_DAVINCI_DM365 && !VIDEO_DM365_ISIF)
depends on VIDEO_V4L2_SUBDEV_API
depends on VIDEO_DAVINCI_VPBE_DISPLAY
select VIDEOBUF2_DMA_CONTIG
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: staging/intel-ipu3: reduce kernel stack usage
On Tue, Mar 5, 2019 at 8:53 AM Sakari Ailus
wrote:
> On Tue, Mar 05, 2019 at 12:25:18AM +, Cao, Bingbu wrote:
> > > struct v4l2_pix_format_mplane *const in =
> > > &q[IPU3_CSS_QUEUE_IN].fmt.mpix;
> > > struct v4l2_pix_format_mplane *const out = @@ -1753,6 +1754,11 @@
> > > int imgu_css_fmt_try(struct imgu_css *css,
> > > &q[IPU3_CSS_QUEUE_VF].fmt.mpix;
> > > int i, s, ret;
> > >
> > > + if (!q) {
> > > + ret = -ENOMEM;
> > > + goto out;
> > > + }
> > [Cao, Bingbu]
> > The goto here is wrong, you can just report an error, and I prefer it is
> > next to the alloc.
>
> I agree, the goto is just not needed.
Should I remove all the gotos then and do an explicit kfree() in each
error path?
I'd prefer not to mix the two styles, as that can lead to subtle mistakes
when the code is refactored again.
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: staging: davinci_vpfe: disallow building with COMPILE_TEST
On Tue, Mar 5, 2019 at 9:05 AM Geert Uytterhoeven wrote: > On Mon, Mar 4, 2019 at 9:30 PM Arnd Bergmann wrote: > > The driver should really call dm365_isif_setup_pinmux() through a callback, > > but it runs platform specific code by itself, which never actually compiled: > > > > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:2: error: > > implicit declaration of function 'davinci_cfg_reg' > > [-Werror,-Wimplicit-function-declaration] > > davinci_cfg_reg(DM365_VIN_CAM_WEN); > > ^ > > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:2: error: > > this function declaration is not a prototype [-Werror,-Wstrict-prototypes] > > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:18: > > error: use of undeclared identifier 'DM365_VIN_CAM_WEN' > > davinci_cfg_reg(DM365_VIN_CAM_WEN); > > ^ > > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2029:18: > > error: use of undeclared identifier 'DM365_VIN_CAM_VD' > > davinci_cfg_reg(DM365_VIN_CAM_VD); > > ^ > > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2030:18: > > error: use of undeclared identifier 'DM365_VIN_CAM_HD' > > davinci_cfg_reg(DM365_VIN_CAM_HD); > > ^ > > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2031:18: > > error: use of undeclared identifier 'DM365_VIN_YIN4_7_EN' > > davinci_cfg_reg(DM365_VIN_YIN4_7_EN); > > ^ > > /git/arm-soc/drivers/staging/media/davinci_vpfe/dm365_isif.c:2032:18: > > error: use of undeclared identifier 'DM365_VIN_YIN0_3_EN' > > davinci_cfg_reg(DM365_VIN_YIN0_3_EN); > > ^ > > 7 errors generated. > > Which tree and which config is this? > This driver compiles fine with m68k/allmodconfig on v5.0? Ah, thanks for checking, I found the real issue now: The Makefile contains a nasty hack that makes it work /almost/ everywhere # Allow building it with COMPILE_TEST on other archs ifndef CONFIG_ARCH_DAVINCI ccflags-y += -I $(srctree)/arch/arm/mach-davinci/include/ endif This is something we probably don't want to do, but it mostly happens to do the right thing for compile testing. The case I ran into is the rare exception of arch/arm/mach-omap1, which has a different mach/mux.h header, so the '#include ' in the driver gets the omap file rather than the davinci file, and then misses the davinci_cfg_reg() declaration and the macros. One way to work around this is to pile on to the hack by adding 'depends on !ARCH_OMAP1'. Should we do that, or is there a better way out? Do we actually still need the staging driver in addition to the one in drivers/media/platform/davinci ? Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: staging/intel-ipu3: reduce kernel stack usage
On Tue, Mar 5, 2019 at 9:47 AM Sakari Ailus
wrote:
> On Tue, Mar 05, 2019 at 09:40:24AM +0100, Arnd Bergmann wrote:
> > On Tue, Mar 5, 2019 at 8:53 AM Sakari Ailus
> > wrote:
> > > On Tue, Mar 05, 2019 at 12:25:18AM +, Cao, Bingbu wrote:
> >
> > > > > struct v4l2_pix_format_mplane *const in =
> > > > > &q[IPU3_CSS_QUEUE_IN].fmt.mpix;
> > > > > struct v4l2_pix_format_mplane *const out = @@ -1753,6 +1754,11 @@
> > > > > int imgu_css_fmt_try(struct imgu_css *css,
> > > > > &q[IPU3_CSS_QUEUE_VF].fmt.mpix;
> > > > > int i, s, ret;
> > > > >
> > > > > + if (!q) {
> > > > > + ret = -ENOMEM;
> > > > > + goto out;
> > > > > + }
> > > > [Cao, Bingbu]
> > > > The goto here is wrong, you can just report an error, and I prefer it
> > > > is next to the alloc.
> > >
> > > I agree, the goto is just not needed.
> >
> > Should I remove all the gotos then and do an explicit kfree() in each
> > error path?
> >
> > I'd prefer not to mix the two styles, as that can lead to subtle mistakes
> > when the code is refactored again.
>
> In this case there's no need for kfree as q is NULL. Goto is often useful
> if you need to do things to undo stuff that was done earlier in the
> function but it's not the case here. I prefer keeping the rest gotos.
Ok, I'll send an updated patch the way you suggested then.
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [v2] media: staging/intel-ipu3: reduce kernel stack usage
The imgu_css_queue structure is too large to be put on the kernel
stack, as we can see in 32-bit builds:
drivers/staging/media/ipu3/ipu3-css.c: In function 'imgu_css_fmt_try':
drivers/staging/media/ipu3/ipu3-css.c:1863:1: error: the frame size of 1172
bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
By dynamically allocating this array, the stack usage goes down to an
acceptable 140 bytes for the same x86-32 configuration.
Fixes: f5f2e4273518 ("media: staging/intel-ipu3: Add css pipeline programming")
Signed-off-by: Arnd Bergmann
---
v2: restructure to use 'return -ENOMEM' instead of goto for failed
allocation.
---
drivers/staging/media/ipu3/ipu3-css.c | 35 ++-
1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/media/ipu3/ipu3-css.c
b/drivers/staging/media/ipu3/ipu3-css.c
index 15ab77e4b766..e7f1898874fd 100644
--- a/drivers/staging/media/ipu3/ipu3-css.c
+++ b/drivers/staging/media/ipu3/ipu3-css.c
@@ -3,6 +3,7 @@
#include
#include
+#include
#include "ipu3-css.h"
#include "ipu3-css-fw.h"
@@ -1744,15 +1745,18 @@ int imgu_css_fmt_try(struct imgu_css *css,
struct v4l2_rect *const bds = &r[IPU3_CSS_RECT_BDS];
struct v4l2_rect *const env = &r[IPU3_CSS_RECT_ENVELOPE];
struct v4l2_rect *const gdc = &r[IPU3_CSS_RECT_GDC];
- struct imgu_css_queue q[IPU3_CSS_QUEUES];
- struct v4l2_pix_format_mplane *const in =
- &q[IPU3_CSS_QUEUE_IN].fmt.mpix;
- struct v4l2_pix_format_mplane *const out =
- &q[IPU3_CSS_QUEUE_OUT].fmt.mpix;
- struct v4l2_pix_format_mplane *const vf =
- &q[IPU3_CSS_QUEUE_VF].fmt.mpix;
+ struct imgu_css_queue *q;
+ struct v4l2_pix_format_mplane *in, *out, *vf;
int i, s, ret;
+ q = kcalloc(IPU3_CSS_QUEUES, sizeof(struct imgu_css_queue), GFP_KERNEL);
+ if (!q)
+ return -ENOMEM;
+
+ in = &q[IPU3_CSS_QUEUE_IN].fmt.mpix;
+ out = &q[IPU3_CSS_QUEUE_OUT].fmt.mpix;
+ vf = &q[IPU3_CSS_QUEUE_VF].fmt.mpix;
+
/* Adjust all formats, get statistics buffer sizes and formats */
for (i = 0; i < IPU3_CSS_QUEUES; i++) {
if (fmts[i])
@@ -1766,7 +1770,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
IPU3_CSS_QUEUE_TO_FLAGS(i))) {
dev_notice(css->dev, "can not initialize queue %s\n",
qnames[i]);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
}
for (i = 0; i < IPU3_CSS_RECTS; i++) {
@@ -1788,7 +1793,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
if (!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_IN]) ||
!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_OUT])) {
dev_warn(css->dev, "required queues are disabled\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
if (!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_OUT])) {
@@ -1829,7 +1835,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
ret = imgu_css_find_binary(css, pipe, q, r);
if (ret < 0) {
dev_err(css->dev, "failed to find suitable binary\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
css->pipes[pipe].bindex = ret;
@@ -1843,7 +1850,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
IPU3_CSS_QUEUE_TO_FLAGS(i))) {
dev_err(css->dev,
"final resolution adjustment failed\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
*fmts[i] = q[i].fmt.mpix;
}
@@ -1859,7 +1867,10 @@ int imgu_css_fmt_try(struct imgu_css *css,
bds->width, bds->height, gdc->width, gdc->height,
out->width, out->height, vf->width, vf->height);
- return 0;
+ ret = 0;
+out:
+ kfree(q);
+ return ret;
}
int imgu_css_fmt_set(struct imgu_css *css,
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [v2] media: staging: davinci_vpfe: disallow building with COMPILE_TEST
The driver should really call dm365_isif_setup_pinmux() through a callback,
but uses a hack to include a davinci specific machine header file when
compile testing instead. This works almost everywhere, but not on the
ARM omap1 platform, which has another header named mach/mux.h. This
causes a build failure:
drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:2: error: implicit
declaration of function 'davinci_cfg_reg'
[-Werror,-Wimplicit-function-declaration]
davinci_cfg_reg(DM365_VIN_CAM_WEN);
^
drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:2: error: this function
declaration is not a prototype [-Werror,-Wstrict-prototypes]
drivers/staging/media/davinci_vpfe/dm365_isif.c:2028:18: error: use of
undeclared identifier 'DM365_VIN_CAM_WEN'
davinci_cfg_reg(DM365_VIN_CAM_WEN);
^
drivers/staging/media/davinci_vpfe/dm365_isif.c:2029:18: error: use of
undeclared identifier 'DM365_VIN_CAM_VD'
davinci_cfg_reg(DM365_VIN_CAM_VD);
^
drivers/staging/media/davinci_vpfe/dm365_isif.c:2030:18: error: use of
undeclared identifier 'DM365_VIN_CAM_HD'
davinci_cfg_reg(DM365_VIN_CAM_HD);
^
drivers/staging/media/davinci_vpfe/dm365_isif.c:2031:18: error: use of
undeclared identifier 'DM365_VIN_YIN4_7_EN'
davinci_cfg_reg(DM365_VIN_YIN4_7_EN);
^
drivers/staging/media/davinci_vpfe/dm365_isif.c:2032:18: error: use of
undeclared identifier 'DM365_VIN_YIN0_3_EN'
davinci_cfg_reg(DM365_VIN_YIN0_3_EN);
^
7 errors generated.
Exclude omap1 from compile-testing, under the assumption that all others
still work.
Fixes: 4907c73deefe ("media: staging: davinci_vpfe: allow building with
COMPILE_TEST")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/davinci_vpfe/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/davinci_vpfe/Kconfig
b/drivers/staging/media/davinci_vpfe/Kconfig
index aea449a8dbf8..76818cc48ddc 100644
--- a/drivers/staging/media/davinci_vpfe/Kconfig
+++ b/drivers/staging/media/davinci_vpfe/Kconfig
@@ -1,7 +1,7 @@
config VIDEO_DM365_VPFE
tristate "DM365 VPFE Media Controller Capture Driver"
depends on VIDEO_V4L2
- depends on (ARCH_DAVINCI_DM365 && !VIDEO_DM365_ISIF) || COMPILE_TEST
+ depends on (ARCH_DAVINCI_DM365 && !VIDEO_DM365_ISIF) || (COMPILE_TEST
&& !ARCH_OMAP1)
depends on VIDEO_V4L2_SUBDEV_API
depends on VIDEO_DAVINCI_VPBE_DISPLAY
select VIDEOBUF2_DMA_CONTIG
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: media: davinci_vpfe: fix large stack usage with clang
clang is unable to optimize the isif_ioctl() in the same way that
gcc does, as it fails to prove that the local copy of
the 'struct vpfe_isif_raw_config' argument is unnecessary:
drivers/staging/media/davinci_vpfe/dm365_isif.c:622:13: error: stack frame size
of 1344 bytes in function 'isif_ioctl' [-Werror,-Wframe-larger-than=]
Marking it as 'const' while passing the data down clearly shows us that
the copy is never modified, and we can skip copying it entirely, which
reduces the stack usage to just eight bytes.
Signed-off-by: Arnd Bergmann
---
.../staging/media/davinci_vpfe/dm365_isif.c | 20 +--
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.c
b/drivers/staging/media/davinci_vpfe/dm365_isif.c
index 0a6d038fcec9..46fd8184fc77 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_isif.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_isif.c
@@ -433,9 +433,9 @@ static int isif_get_params(struct v4l2_subdev *sd, void
*params)
return 0;
}
-static int isif_validate_df_csc_params(struct vpfe_isif_df_csc *df_csc)
+static int isif_validate_df_csc_params(const struct vpfe_isif_df_csc *df_csc)
{
- struct vpfe_isif_color_space_conv *csc;
+ const struct vpfe_isif_color_space_conv *csc;
int err = -EINVAL;
int i;
@@ -481,7 +481,7 @@ static int isif_validate_df_csc_params(struct
vpfe_isif_df_csc *df_csc)
#define DM365_ISIF_MAX_DFCMEM0 0x1fff
#define DM365_ISIF_MAX_DFCMEM1 0x1fff
-static int isif_validate_dfc_params(struct vpfe_isif_dfc *dfc)
+static int isif_validate_dfc_params(const struct vpfe_isif_dfc *dfc)
{
int err = -EINVAL;
int i;
@@ -532,7 +532,7 @@ static int isif_validate_dfc_params(struct vpfe_isif_dfc
*dfc)
#define DM365_ISIF_MAX_CLVSV 0x1fff
#define DM365_ISIF_MAX_HEIGHT_BLACK_REGION 0x1fff
-static int isif_validate_bclamp_params(struct vpfe_isif_black_clamp *bclamp)
+static int isif_validate_bclamp_params(const struct vpfe_isif_black_clamp
*bclamp)
{
int err = -EINVAL;
@@ -580,7 +580,7 @@ static int isif_validate_bclamp_params(struct
vpfe_isif_black_clamp *bclamp)
}
static int
-isif_validate_raw_params(struct vpfe_isif_raw_config *params)
+isif_validate_raw_params(const struct vpfe_isif_raw_config *params)
{
int ret;
@@ -593,20 +593,18 @@ isif_validate_raw_params(struct vpfe_isif_raw_config
*params)
return isif_validate_bclamp_params(¶ms->bclamp);
}
-static int isif_set_params(struct v4l2_subdev *sd, void *params)
+static int isif_set_params(struct v4l2_subdev *sd, const struct
vpfe_isif_raw_config *params)
{
struct vpfe_isif_device *isif = v4l2_get_subdevdata(sd);
- struct vpfe_isif_raw_config isif_raw_params;
int ret = -EINVAL;
/* only raw module parameters can be set through the IOCTL */
if (isif->formats[ISIF_PAD_SINK].code != MEDIA_BUS_FMT_SGRBG12_1X12)
return ret;
- memcpy(&isif_raw_params, params, sizeof(isif_raw_params));
- if (!isif_validate_raw_params(&isif_raw_params)) {
- memcpy(&isif->isif_cfg.bayer.config_params, &isif_raw_params,
- sizeof(isif_raw_params));
+ if (!isif_validate_raw_params(params)) {
+ memcpy(&isif->isif_cfg.bayer.config_params, params,
+ sizeof(*params));
ret = 0;
}
return ret;
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: media: imx7-mipi-csis: fix debugfs compilation
When CONFIG_DEBUGFS is enabled, we get a warning about an
incorrect section annotation that can lead to undefined
behavior:
WARNING: vmlinux.o(.text+0xd3c7c4): Section mismatch in reference from the
function mipi_csis_probe() to the function .init.text:mipi_csis_debugfs_init()
The function mipi_csis_probe() references
the function __init mipi_csis_debugfs_init().
This is often because mipi_csis_probe lacks a __init
annotation or the annotation of mipi_csis_debugfs_init is wrong.
The same function for an unknown reason has a different
version for !CONFIG_DEBUGFS, which does not have this problem,
but behaves the same way otherwise (it does nothing when debugfs
is disabled).
Consolidate the two versions, using the correct section from
one version, and the implementation from the other.
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/imx/imx7-mipi-csis.c | 16 ++--
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c
b/drivers/staging/media/imx/imx7-mipi-csis.c
index 2ddcc42ab8ff..001ce369ec45 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -9,6 +9,7 @@
*/
#include
+#include
#include
#include
#include
@@ -889,8 +890,6 @@ static int mipi_csis_subdev_init(struct v4l2_subdev
*mipi_sd,
return ret;
}
-#ifdef CONFIG_DEBUG_FS
-#include
static int mipi_csis_dump_regs_show(struct seq_file *m, void *private)
{
@@ -900,7 +899,7 @@ static int mipi_csis_dump_regs_show(struct seq_file *m,
void *private)
}
DEFINE_SHOW_ATTRIBUTE(mipi_csis_dump_regs);
-static int __init_or_module mipi_csis_debugfs_init(struct csi_state *state)
+static int mipi_csis_debugfs_init(struct csi_state *state)
{
struct dentry *d;
@@ -934,17 +933,6 @@ static void mipi_csis_debugfs_exit(struct csi_state *state)
debugfs_remove_recursive(state->debugfs_root);
}
-#else
-static int mipi_csis_debugfs_init(struct csi_state *state __maybe_unused)
-{
- return 0;
-}
-
-static void mipi_csis_debugfs_exit(struct csi_state *state __maybe_unused)
-{
-}
-#endif
-
static int mipi_csis_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] exfat stopusing CONFIG_FAT_DEFAULT_IOCHARSET
When CONFIG_VFAT_FS is disabled, the reference to CONFIG_FAT_DEFAULT_IOCHARSET causes a link failure: drivers/staging/exfat/exfat_super.c:46:41: error: use of undeclared identifier 'CONFIG_FAT_DEFAULT_IOCHARSET' static char exfat_default_iocharset[] = CONFIG_FAT_DEFAULT_IOCHARSET; I could not figure out why the correct code is commented out, but using that fixes the problem. Signed-off-by: Arnd Bergmann --- drivers/staging/exfat/exfat_super.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/staging/exfat/exfat_super.c b/drivers/staging/exfat/exfat_super.c index 3934b120e1dd..37e78620723f 100644 --- a/drivers/staging/exfat/exfat_super.c +++ b/drivers/staging/exfat/exfat_super.c @@ -39,11 +39,8 @@ static struct kmem_cache *exfat_inode_cachep; -// FIXME use commented lines -// static int exfat_default_codepage = CONFIG_EXFAT_DEFAULT_CODEPAGE; -// static char exfat_default_iocharset[] = CONFIG_EXFAT_DEFAULT_IOCHARSET; -static int exfat_default_codepage = CONFIG_FAT_DEFAULT_CODEPAGE; -static char exfat_default_iocharset[] = CONFIG_FAT_DEFAULT_IOCHARSET; +static int exfat_default_codepage = CONFIG_EXFAT_DEFAULT_CODEPAGE; +static char exfat_default_iocharset[] = CONFIG_EXFAT_DEFAULT_IOCHARSET; #define INC_IVERSION(x) (inode_inc_iversion(x)) #define GET_IVERSION(x) (inode_peek_iversion_raw(x)) -- 2.20.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] exfat: stop using 32-bit time_t
time_t suffers from overflow problems and should not be used.
In exfat, it is currently used in two open-coded time64_to_tm()
implementations. Changes those to use the existing function instead.
Signed-off-by: Arnd Bergmann
---
drivers/staging/exfat/exfat_super.c | 164 +++-
1 file changed, 38 insertions(+), 126 deletions(-)
diff --git a/drivers/staging/exfat/exfat_super.c
b/drivers/staging/exfat/exfat_super.c
index 37e78620723f..7fd56654498f 100644
--- a/drivers/staging/exfat/exfat_super.c
+++ b/drivers/staging/exfat/exfat_super.c
@@ -55,129 +55,65 @@ static int exfat_write_inode(struct inode *inode,
static void exfat_write_super(struct super_block *sb);
#define UNIX_SECS_1980315532800L
-
-#if BITS_PER_LONG == 64
#define UNIX_SECS_21084354819200L
-#endif
-
-/* days between 1.1.70 and 1.1.80 (2 leap days) */
-#define DAYS_DELTA_DECADE(365 * 10 + 2)
-/* 120 (2100 - 1980) isn't leap year */
-#define NO_LEAP_YEAR_2100(120)
-#define IS_LEAP_YEAR(y)(!((y) & 0x3) && (y) != NO_LEAP_YEAR_2100)
-
-#define SECS_PER_MIN(60)
-#define SECS_PER_HOUR (60 * SECS_PER_MIN)
-#define SECS_PER_DAY(24 * SECS_PER_HOUR)
-
-#define MAKE_LEAP_YEAR(leap_year, year) \
- do {\
- if (unlikely(year > NO_LEAP_YEAR_2100)) \
- leap_year = ((year + 3) / 4) - 1; \
- else\
- leap_year = ((year + 3) / 4); \
- } while (0)
-
-/* Linear day numbers of the respective 1sts in non-leap years. */
-static time_t accum_days_in_year[] = {
- /* Jan Feb Mar Apr May Jun Jul Aug Sep Oct Nov Dec */
- 0, 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 0, 0, 0,
-};
/* Convert a FAT time/date pair to a UNIX date (seconds since 1 1 70). */
static void exfat_time_fat2unix(struct exfat_sb_info *sbi,
struct timespec64 *ts, struct date_time_t *tp)
{
- time_t year = tp->Year;
- time_t ld;
-
- MAKE_LEAP_YEAR(ld, year);
-
- if (IS_LEAP_YEAR(year) && (tp->Month) > 2)
- ld++;
-
- ts->tv_sec = tp->Second +
-tp->Minute * SECS_PER_MIN +
-tp->Hour * SECS_PER_HOUR +
-(ld + accum_days_in_year[(tp->Month)] +
- (tp->Day - 1)) * SECS_PER_DAY +
-(year * 365 + DAYS_DELTA_DECADE) * SECS_PER_DAY +
-sys_tz.tz_minuteswest * SECS_PER_MIN;
+ ts->tv_sec = mktime64(tp->Year + 1980, tp->Month + 1, tp->Day,
+ tp->Hour, tp->Minute, tp->Second);
- ts->tv_nsec = 0;
+ ts->tv_nsec = tp->MilliSecond * NSEC_PER_MSEC;
}
/* Convert linear UNIX date to a FAT time/date pair. */
static void exfat_time_unix2fat(struct exfat_sb_info *sbi,
struct timespec64 *ts, struct date_time_t *tp)
{
- time_t second = ts->tv_sec;
- time_t day, month, year;
- time_t ld;
+ time64_t second = ts->tv_sec;
+ struct tm tm;
- second -= sys_tz.tz_minuteswest * SECS_PER_MIN;
+ time64_to_tm(second, 0, &tm);
- /* Jan 1 GMT 00:00:00 1980. But what about another time zone? */
if (second < UNIX_SECS_1980) {
- tp->Second = 0;
- tp->Minute = 0;
- tp->Hour = 0;
- tp->Day = 1;
- tp->Month = 1;
- tp->Year = 0;
+ tp->MilliSecond = 0;
+ tp->Second = 0;
+ tp->Minute = 0;
+ tp->Hour= 0;
+ tp->Day = 1;
+ tp->Month = 1;
+ tp->Year= 0;
return;
}
-#if (BITS_PER_LONG == 64)
+
if (second >= UNIX_SECS_2108) {
- tp->Second = 59;
- tp->Minute = 59;
- tp->Hour = 23;
- tp->Day = 31;
- tp->Month = 12;
- tp->Year = 127;
+ tp->MilliSecond = 999;
+ tp->Second = 59;
+ tp->Minute = 59;
+ tp->Hour= 23;
+ tp->Day = 31;
+ tp->Month = 12;
+ tp->Year= 127;
return;
}
-#endif
- day = second / SECS_PER_DAY - DAYS_DELTA_DECADE;
- year = day / 365;
- MAKE_LEAP_YEAR(ld, year);
- if (year * 365 + ld > day)
- year--;
-
- MAKE_LEAP_YEAR(ld, year);
- day -= year * 365 + ld;
- if (IS_LEAP_YEAR(year) && day == accum_days_in_year[3]) {
- month = 2;
- }
Re: [PATCH] media: staging: tegra-vde: Fix build error
On Thu, Jul 25, 2019 at 2:24 PM Dmitry Osipenko wrote:
>
> 25.07.2019 5:41, YueHaibing пишет:
> > If IOMMU_SUPPORT is not set, and COMPILE_TEST is y,
> > IOMMU_IOVA may be set to m. So building will fails:
> >
> > drivers/staging/media/tegra-vde/iommu.o: In function `tegra_vde_iommu_map':
> > iommu.c:(.text+0x41): undefined reference to `alloc_iova'
> > iommu.c:(.text+0x56): undefined reference to `__free_iova'
> >
> > Select IOMMU_IOVA while COMPILE_TEST is set to fix this.
> > @@ -3,7 +3,7 @@ config TEGRA_VDE
> > tristate "NVIDIA Tegra Video Decoder Engine driver"
> > depends on ARCH_TEGRA || COMPILE_TEST
> > select DMA_SHARED_BUFFER
> > - select IOMMU_IOVA if IOMMU_SUPPORT
> > + select IOMMU_IOVA if (IOMMU_SUPPORT || COMPILE_TEST)
> > select SRAM
> > help
> > Say Y here to enable support for the NVIDIA Tegra video decoder
> >
>
> This results in missing the case of compile-testing !IOMMU_IOVA for the
> driver, but probably that's not a big deal.
>
> Acked-by: Dmitry Osipenko
I don't know what happened here, but the patch from YueHaibing caused this
error for me, which is very much like the problem it was meant to fix:
drivers/gpu/host1x/dev.o: In function `host1x_probe':
dev.c:(.text+0x1734): undefined reference to `put_iova_domain'
dev.c:(.text+0x1744): undefined reference to `iova_cache_put'
drivers/gpu/host1x/dev.o: In function `host1x_remove':
dev.c:(.text+0x1894): undefined reference to `put_iova_domain'
dev.c:(.text+0x1898): undefined reference to `iova_cache_put'
drivers/gpu/host1x/cdma.o: In function `host1x_cdma_init':
cdma.c:(.text+0x5d0): undefined reference to `alloc_iova'
cdma.c:(.text+0x61c): undefined reference to `__free_iova'
drivers/gpu/host1x/cdma.o: In function `host1x_cdma_deinit':
cdma.c:(.text+0x6c8): undefined reference to `free_iova'
drivers/gpu/host1x/job.o: In function `host1x_job_pin':
job.c:(.text+0x514): undefined reference to `alloc_iova'
job.c:(.text+0x528): undefined reference to `__free_iova'
drivers/gpu/host1x/job.o: In function `host1x_job_unpin':
job.c:(.text+0x5bc): undefined reference to `free_iova'
After reverthing commit 6b2265975239 ("media: staging:
tegra-vde: Fix build error"), I can no longer reproduce the
issue.
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/16] drivers: y2038 updates
These are updates to devidce drivers and file systems that for some reason or another were not included in the kernel in the previous y2038 series. I've gone through all users of time_t again to make sure the kernel is in a long-term maintainable state. Posting these as a series for better organization, but each change here is applicable standalone. Please merge, review, ack/nack etc as you see fit. My plan is to include any patches that don't get a reply this time around in a future pull request, probably for linux-5.6. As mentioned before, the full series of 90 patches is available at https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=y2038-endgame Arnd Arnd Bergmann (16): staging: exfat: use prandom_u32() for i_generation fat: use prandom_u32() for i_generation net: sock: use __kernel_old_timespec instead of timespec dlm: use SO_SNDTIMEO_NEW instead of SO_SNDTIMEO_OLD xtensa: ISS: avoid struct timeval um: ubd: use 64-bit time_t where possible acct: stop using get_seconds() tsacct: add 64-bit btime field netfilter: nft_meta: use 64-bit time arithmetic packet: clarify timestamp overflow quota: avoid time_t in v1_disk_dqblk definition hostfs: pass 64-bit timestamps to/from user space hfs/hfsplus: use 64-bit inode timestamps drm/msm: avoid using 'timespec' drm/etnaviv: use ktime_t for timeouts firewire: ohci: stop using get_seconds() for BUS_TIME arch/um/drivers/cow.h | 2 +- arch/um/drivers/cow_user.c| 7 +++-- arch/um/drivers/ubd_kern.c| 10 +++ arch/um/include/shared/os.h | 2 +- arch/um/os-Linux/file.c | 2 +- .../platforms/iss/include/platform/simcall.h | 4 +-- drivers/firewire/ohci.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_drv.c | 19 ++--- drivers/gpu/drm/etnaviv/etnaviv_drv.h | 21 ++ drivers/gpu/drm/etnaviv/etnaviv_gem.c | 5 ++-- drivers/gpu/drm/etnaviv/etnaviv_gem.h | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 +-- drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 4 +-- drivers/gpu/drm/msm/msm_drv.h | 3 +- drivers/staging/exfat/exfat_super.c | 4 +-- fs/dlm/lowcomms.c | 6 ++-- fs/fat/inode.c| 3 +- fs/hfs/hfs_fs.h | 26 + fs/hfs/inode.c| 4 +-- fs/hfsplus/hfsplus_fs.h | 26 + fs/hfsplus/inode.c| 12 fs/hostfs/hostfs.h| 22 +-- fs/hostfs/hostfs_kern.c | 15 ++ fs/quota/quotaio_v1.h | 6 ++-- include/linux/skbuff.h| 7 +++-- include/uapi/linux/acct.h | 2 ++ include/uapi/linux/taskstats.h| 6 +++- kernel/acct.c | 4 ++- kernel/tsacct.c | 9 -- net/compat.c | 2 +- net/ipv4/tcp.c| 28 +++ net/netfilter/nft_meta.c | 10 +++ net/packet/af_packet.c| 27 +++--- net/socket.c | 2 +- 34 files changed, 184 insertions(+), 124 deletions(-) -- 2.20.0 Cc: jd...@addtoit.com Cc: rich...@nod.at Cc: jcmvb...@gmail.com Cc: stef...@s5r6.in-berlin.de Cc: l.st...@pengutronix.de Cc: linux+etna...@armlinux.org.uk Cc: christian.gmei...@gmail.com Cc: airl...@linux.ie Cc: dan...@ffwll.ch Cc: robdcl...@gmail.com Cc: s...@poorly.run Cc: valdis.kletni...@vt.edu Cc: gre...@linuxfoundation.org Cc: ccaul...@redhat.com Cc: teigl...@redhat.com Cc: hirof...@mail.parknet.co.jp Cc: j...@suse.com Cc: da...@davemloft.net Cc: eduma...@google.com Cc: pa...@netfilter.org Cc: kad...@netfilter.org Cc: f...@strlen.de Cc: will...@google.com Cc: v...@zeniv.linux.org.uk Cc: rfont...@redhat.com Cc: t...@linutronix.de Cc: linux...@lists.infradead.org Cc: linux-ker...@vger.kernel.org Cc: linux1394-de...@lists.sourceforge.net Cc: etna...@lists.freedesktop.org Cc: dri-de...@lists.freedesktop.org> Cc: linux-arm-...@vger.kernel.org> Cc: freedr...@lists.freedesktop.org> Cc: de...@driverdev.osuosl.org> Cc: cluster-de...@redhat.com> Cc: linux-fsde...@vger.kernel.org> Cc: net...@vger.kernel.org> Cc: netfilter-de...@vger.kernel.org> Cc: coret...@netfilter.org> ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/16] staging: exfat: use prandom_u32() for i_generation
Similar to commit 46c9a946d766 ("shmem: use monotonic time for i_generation")
we should not use the deprecated get_seconds() interface for i_generation.
prandom_u32() is the replacement used in other file systems.
Signed-off-by: Arnd Bergmann
---
drivers/staging/exfat/exfat_super.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/exfat/exfat_super.c
b/drivers/staging/exfat/exfat_super.c
index 3b2b0ceb7297..da76c607f589 100644
--- a/drivers/staging/exfat/exfat_super.c
+++ b/drivers/staging/exfat/exfat_super.c
@@ -26,7 +26,7 @@
#include
#include
#include
-
+#include
#include
#include
#include
@@ -3314,7 +3314,7 @@ static int exfat_fill_inode(struct inode *inode, struct
file_id_t *fid)
inode->i_uid = sbi->options.fs_uid;
inode->i_gid = sbi->options.fs_gid;
INC_IVERSION(inode);
- inode->i_generation = get_seconds();
+ inode->i_generation = prandom_u32();
if (info.Attr & ATTR_SUBDIR) { /* directory */
inode->i_generation &= ~1;
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 0/4] Correct vendor-prefix and document device isl29028
On Saturday 08 November 2014, Darshana Padmadas wrote: > Patchset documents correct and the deprecated vendor-prefix found by > checkpatch > warning and also documents information of device isl29028 for compatibility. > Patchset also includes corrected vendor-prefix and device name in compatible > property for files with checkpatch warning of undocumented string > "isil,isl29028". > Since nobody else has picked these up, I've put them into the arm-soc tree in the next/dt branch now. Thanks and sorry for the delay, Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: fix coding style error
On Wednesday 14 January 2015 09:20:42 Patrick Boettcher wrote:
> diff --git a/drivers/staging/android/sync_debug.c
> b/drivers/staging/android/sync_debug.c
> index 1532a86..9a2aaf8 100644
> --- a/drivers/staging/android/sync_debug.c
> +++ b/drivers/staging/android/sync_debug.c
> @@ -96,7 +96,8 @@ static void sync_print_pt(struct seq_file *s, struct
> sync_pt *pt, bool fence)
>sync_status_str(status));
>
> if (status <= 0) {
> - struct timespec64 ts64 =
> ktime_to_timespec64(pt->base.timestamp);
> + struct timespec64 ts64 =
> + ktime_to_timespec64(pt->base.timestamp);
>
> seq_printf(s, "@%lld.%09ld", (s64)ts64.tv_sec, ts64.tv_nsec);
> }
>
You are not really improving readability by just wrapping the line
arbitrarily though.
If you're offended by the long line, better rework the code so the long
line is not needed.
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 00/26] compat_ioctl: cleanups
Hi Al, It took me way longer than I had hoped to revisit this series, see https://lore.kernel.org/lkml/20180912150142.157913-1-a...@arndb.de/ for the previously posted version. I've come to the point where all conversion handlers and most COMPATIBLE_IOCTL() entries are gone from this file, but for now, this series only has the parts that have either been reviewed previously, or that are simple enough to include. The main missing piece is the SG_IO/SG_GET_REQUEST_TABLE conversion. I'll post the patches I made for that later, as they need more testing and review from the scsi maintainers. I hope you can still take these for the coming merge window, unless new problems come up. Arnd Arnd Bergmann (26): compat_ioctl: pppoe: fix PPPOEIOCSFWD handling compat_ioctl: move simple ppp command handling into driver compat_ioctl: avoid unused function warning for do_ioctl compat_ioctl: move PPPIOCSCOMPRESS32 to ppp-generic.c compat_ioctl: move PPPIOCSPASS32/PPPIOCSACTIVE32 to ppp_generic.c compat_ioctl: handle PPPIOCGIDLE for 64-bit time_t compat_ioctl: move rtc handling into rtc-dev.c compat_ioctl: add compat_ptr_ioctl() compat_ioctl: move drivers to compat_ptr_ioctl compat_ioctl: use correct compat_ptr() translation in drivers ceph: fix compat_ioctl for ceph_dir_operations compat_ioctl: move more drivers to compat_ptr_ioctl compat_ioctl: move tape handling into drivers compat_ioctl: move ATYFB_CLK handling to atyfb driver compat_ioctl: move isdn/capi ioctl translation into driver compat_ioctl: move rfcomm handlers into driver compat_ioctl: move hci_sock handlers into driver compat_ioctl: remove HCIUART handling compat_ioctl: remove HIDIO translation compat_ioctl: remove translation for sound ioctls compat_ioctl: remove IGNORE_IOCTL() compat_ioctl: remove /dev/random commands compat_ioctl: remove joystick ioctl translation compat_ioctl: remove PCI ioctl translation compat_ioctl: remove /dev/raw ioctl translation compat_ioctl: remove last RAID handling code Documentation/networking/ppp_generic.txt| 2 + arch/um/drivers/hostaudio_kern.c| 1 + drivers/android/binder.c| 2 +- drivers/char/ppdev.c| 12 +- drivers/char/random.c | 1 + drivers/char/tpm/tpm_vtpm_proxy.c | 12 +- drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +- drivers/dma-buf/dma-buf.c | 4 +- drivers/dma-buf/sw_sync.c | 2 +- drivers/dma-buf/sync_file.c | 2 +- drivers/firewire/core-cdev.c| 12 +- drivers/gpu/drm/amd/amdkfd/kfd_chardev.c| 2 +- drivers/hid/hidraw.c| 4 +- drivers/hid/usbhid/hiddev.c | 11 +- drivers/hwtracing/stm/core.c| 12 +- drivers/ide/ide-tape.c | 31 +- drivers/iio/industrialio-core.c | 2 +- drivers/infiniband/core/uverbs_main.c | 4 +- drivers/isdn/capi/capi.c| 31 + drivers/isdn/i4l/isdn_ppp.c | 14 +- drivers/media/rc/lirc_dev.c | 4 +- drivers/mfd/cros_ec_dev.c | 4 +- drivers/misc/cxl/flash.c| 8 +- drivers/misc/genwqe/card_dev.c | 23 +- drivers/misc/mei/main.c | 22 +- drivers/misc/vmw_vmci/vmci_host.c | 2 +- drivers/mtd/ubi/cdev.c | 36 +- drivers/net/ppp/ppp_generic.c | 99 +++- drivers/net/ppp/pppoe.c | 7 + drivers/net/ppp/pptp.c | 3 + drivers/net/tap.c | 12 +- drivers/nvdimm/bus.c| 4 +- drivers/nvme/host/core.c| 2 +- drivers/pci/switch/switchtec.c | 2 +- drivers/platform/x86/wmi.c | 2 +- drivers/rpmsg/rpmsg_char.c | 4 +- drivers/rtc/dev.c | 13 +- drivers/rtc/rtc-vr41xx.c| 10 + drivers/s390/char/tape_char.c | 41 +- drivers/sbus/char/display7seg.c | 2 +- drivers/sbus/char/envctrl.c | 4 +- drivers/scsi/3w-.c | 4 +- drivers/scsi/cxlflash/main.c| 2 +- drivers/scsi/esas2r/esas2r_main.c | 2 +- drivers/scsi/megaraid/megaraid_mm.c | 28 +- drivers/scsi/osst.c | 34 +- drivers/scsi/pmcraid.c | 4 +- drivers/scsi/st.c | 35 +- drivers/staging/android/ion/ion.c | 4 +- drivers/staging/pi433/pi433_if.c| 12 +- drivers/staging/vme/devices/vme_user.c | 2 +- drivers/tee/tee_core.c | 2 +- drivers/usb/class/cdc-wdm.c | 2 +- drivers/usb/class/usbtmc.c | 4 +- drivers/usb/co
[PATCH v3 09/26] compat_ioctl: move drivers to compat_ptr_ioctl
Each of these drivers has a copy of the same trivial helper function to
convert the pointer argument and then call the native ioctl handler.
We now have a generic implementation of that, so use it.
Acked-by: Greg Kroah-Hartman
Reviewed-by: Jarkko Sakkinen
Reviewed-by: Jason Gunthorpe
Signed-off-by: Arnd Bergmann
---
drivers/char/ppdev.c | 12 +-
drivers/char/tpm/tpm_vtpm_proxy.c | 12 +-
drivers/firewire/core-cdev.c | 12 +-
drivers/hid/usbhid/hiddev.c | 11 +
drivers/hwtracing/stm/core.c | 12 +-
drivers/misc/mei/main.c | 22 +
drivers/mtd/ubi/cdev.c| 36 +++-
drivers/net/tap.c | 12 +-
drivers/staging/pi433/pi433_if.c | 12 +-
drivers/usb/core/devio.c | 16 +
drivers/vfio/vfio.c | 39 +++
drivers/vhost/net.c | 12 +-
drivers/vhost/scsi.c | 12 +-
drivers/vhost/test.c | 12 +-
drivers/vhost/vsock.c | 12 +-
fs/fat/file.c | 13 +--
16 files changed, 20 insertions(+), 237 deletions(-)
diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index 1ae77b41050a..e96c8d9623e0 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -674,14 +674,6 @@ static long pp_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
return ret;
}
-#ifdef CONFIG_COMPAT
-static long pp_compat_ioctl(struct file *file, unsigned int cmd,
- unsigned long arg)
-{
- return pp_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
static int pp_open(struct inode *inode, struct file *file)
{
unsigned int minor = iminor(inode);
@@ -790,9 +782,7 @@ static const struct file_operations pp_fops = {
.write = pp_write,
.poll = pp_poll,
.unlocked_ioctl = pp_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = pp_compat_ioctl,
-#endif
+ .compat_ioctl = compat_ptr_ioctl,
.open = pp_open,
.release= pp_release,
};
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c
b/drivers/char/tpm/tpm_vtpm_proxy.c
index d74f3de74ae6..fb845f0a430b 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -675,20 +675,10 @@ static long vtpmx_fops_ioctl(struct file *f, unsigned int
ioctl,
}
}
-#ifdef CONFIG_COMPAT
-static long vtpmx_fops_compat_ioctl(struct file *f, unsigned int ioctl,
- unsigned long arg)
-{
- return vtpmx_fops_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
-}
-#endif
-
static const struct file_operations vtpmx_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = vtpmx_fops_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = vtpmx_fops_compat_ioctl,
-#endif
+ .compat_ioctl = compat_ptr_ioctl,
.llseek = noop_llseek,
};
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 16a7045736a9..fb934680fdd3 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1659,14 +1659,6 @@ static long fw_device_op_ioctl(struct file *file,
return dispatch_ioctl(file->private_data, cmd, (void __user *)arg);
}
-#ifdef CONFIG_COMPAT
-static long fw_device_op_compat_ioctl(struct file *file,
- unsigned int cmd, unsigned long arg)
-{
- return dispatch_ioctl(file->private_data, cmd, compat_ptr(arg));
-}
-#endif
-
static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma)
{
struct client *client = file->private_data;
@@ -1808,7 +1800,5 @@ const struct file_operations fw_device_ops = {
.mmap = fw_device_op_mmap,
.release= fw_device_op_release,
.poll = fw_device_op_poll,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = fw_device_op_compat_ioctl,
-#endif
+ .compat_ioctl = compat_ptr_ioctl,
};
diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index a746017fac17..ef4a1cd389d6 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -855,13 +855,6 @@ static long hiddev_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
return r;
}
-#ifdef CONFIG_COMPAT
-static long hiddev_compat_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
-{
- return hiddev_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
static const struct file_operations hiddev_fops = {
.owner =THIS_MODULE,
.read = hiddev_read,
@@ -871,9 +864,7 @@ static const struct file_operations hiddev_fops = {
.release = hiddev_release,
.unlocked_ioctl = hiddev_ioctl,
.fasync = hiddev_fasync,
-#ifdef CONFIG_COMPAT
- .com
[PATCH v3 12/26] compat_ioctl: move more drivers to compat_ptr_ioctl
The .ioctl and .compat_ioctl file operations have the same prototype so
they can both point to the same function, which works great almost all
the time when all the commands are compatible.
One exception is the s390 architecture, where a compat pointer is only
31 bit wide, and converting it into a 64-bit pointer requires calling
compat_ptr(). Most drivers here will ever run in s390, but since we now
have a generic helper for it, it's easy enough to use it consistently.
I double-checked all these drivers to ensure that all ioctl arguments
are used as pointers or are ignored, but are not interpreted as integer
values.
Acked-by: Jason Gunthorpe
Acked-by: Daniel Vetter
Acked-by: Mauro Carvalho Chehab
Acked-by: Greg Kroah-Hartman
Acked-by: David Sterba
Acked-by: Darren Hart (VMware)
Acked-by: Jonathan Cameron
Acked-by: Bjorn Andersson
Signed-off-by: Arnd Bergmann
---
drivers/android/binder.c| 2 +-
drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +-
drivers/dma-buf/dma-buf.c | 4 +---
drivers/dma-buf/sw_sync.c | 2 +-
drivers/dma-buf/sync_file.c | 2 +-
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c| 2 +-
drivers/hid/hidraw.c| 4 +---
drivers/iio/industrialio-core.c | 2 +-
drivers/infiniband/core/uverbs_main.c | 4 ++--
drivers/media/rc/lirc_dev.c | 4 +---
drivers/mfd/cros_ec_dev.c | 4 +---
drivers/misc/vmw_vmci/vmci_host.c | 2 +-
drivers/nvdimm/bus.c| 4 ++--
drivers/nvme/host/core.c| 2 +-
drivers/pci/switch/switchtec.c | 2 +-
drivers/platform/x86/wmi.c | 2 +-
drivers/rpmsg/rpmsg_char.c | 4 ++--
drivers/sbus/char/display7seg.c | 2 +-
drivers/sbus/char/envctrl.c | 4 +---
drivers/scsi/3w-.c | 4 +---
drivers/scsi/cxlflash/main.c| 2 +-
drivers/scsi/esas2r/esas2r_main.c | 2 +-
drivers/scsi/pmcraid.c | 4 +---
drivers/staging/android/ion/ion.c | 4 +---
drivers/staging/vme/devices/vme_user.c | 2 +-
drivers/tee/tee_core.c | 2 +-
drivers/usb/class/cdc-wdm.c | 2 +-
drivers/usb/class/usbtmc.c | 4 +---
drivers/virt/fsl_hypervisor.c | 2 +-
fs/btrfs/super.c| 2 +-
fs/ceph/dir.c | 2 +-
fs/ceph/file.c | 2 +-
fs/fuse/dev.c | 2 +-
fs/notify/fanotify/fanotify_user.c | 2 +-
fs/userfaultfd.c| 2 +-
net/rfkill/core.c | 2 +-
36 files changed, 39 insertions(+), 57 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 4b9c7ca492e6..48109ade7234 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5998,7 +5998,7 @@ const struct file_operations binder_fops = {
.owner = THIS_MODULE,
.poll = binder_poll,
.unlocked_ioctl = binder_ioctl,
- .compat_ioctl = binder_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
.mmap = binder_mmap,
.open = binder_open,
.flush = binder_flush,
diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
index abc7a7f64d64..ef0e482ee04f 100644
--- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
+++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
@@ -68,7 +68,7 @@ static long adf_ctl_ioctl(struct file *fp, unsigned int cmd,
unsigned long arg);
static const struct file_operations adf_ctl_ops = {
.owner = THIS_MODULE,
.unlocked_ioctl = adf_ctl_ioctl,
- .compat_ioctl = adf_ctl_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
};
struct adf_ctl_drv_info {
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 7c858020d14b..0cb336fe6324 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,9 +325,7 @@ static const struct file_operations dma_buf_fops = {
.llseek = dma_buf_llseek,
.poll = dma_buf_poll,
.unlocked_ioctl = dma_buf_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = dma_buf_ioctl,
-#endif
+ .compat_ioctl = compat_ptr_ioctl,
};
/*
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 32dcf7b4c935..411de6a8a0ad 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -419,5 +419,5 @@ const struct file_operations sw_sync_debugfs_fops = {
.open = sw_sync_debugfs_open,
.release= sw_sync_debugfs_release,
.unlocked_ioctl = sw_sync_ioctl,
- .compat_ioctl = sw_sync_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
};
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 4f6305ca52c8..0949f91eb85f 1
Re: [PATCH v3 00/26] compat_ioctl: cleanups
On Wed, Apr 17, 2019 at 12:33 AM Douglas Gilbert wrote:
>
> On 2019-04-16 4:19 p.m., Arnd Bergmann wrote:
> > Hi Al,
> >
> > It took me way longer than I had hoped to revisit this series, see
> > https://lore.kernel.org/lkml/20180912150142.157913-1-a...@arndb.de/
> > for the previously posted version.
> >
> > I've come to the point where all conversion handlers and most
> > COMPATIBLE_IOCTL() entries are gone from this file, but for
> > now, this series only has the parts that have either been reviewed
> > previously, or that are simple enough to include.
> >
> > The main missing piece is the SG_IO/SG_GET_REQUEST_TABLE conversion.
> > I'll post the patches I made for that later, as they need more
> > testing and review from the scsi maintainers.
>
> Perhaps you could look at the document in this url:
> http://sg.danny.cz/sg/sg_v40.html
>
> It is work-in-progress to modernize the SCSI generic driver. It
> extends ioctl(sg_fd, SG_IO, &pt_obj) to additionally accept the sg v4
> interface as defined in include/uapi/linux/bsg.h . Currently only the
> bsg driver uses the sg v4 interface. Since struct sg_io_v4 is all
> explicitly sized integers, I'm guessing it is immune "compat" problems.
> [I can see no reference to bsg nor struct sg_io_v4 in the current
> fs/compat_ioctl.c file.]
Ok, I've taken a brief look at your series now. Unfortunately it clashes
quite hard with my series, but it's probably for the better to have your
stuff get merged first.
A few (unsorted) comments from going through your patches:
- the added ioctls are all compatible when using the v4 structures
and mostly don't need handlers for compat mode, but they need to be
called from .compat_ioctl to actually be usable in compat mode.
With my patches you get that.
- One exception for the v4 layout is the use of iovec pointers, as
'struct iovec' is incompatible. We should probably merge the
generic compat_import_iovec() into import_iovec() with a
'in_compat_syscall()' check, which would be helpful in general.
bsg.c does not iovec, so it is not affected by this at the moment,
maybe it would be better to stay compatible with that and also
not support them in sg.c?
- Is there a need for the new sg_ioctl_iosubmit/sg_ioctl_ioreceive
to support the v3 structures? Those are /not/ compatible, so
you need extra code to handle the v3-compat layout as well.
Supporting only v4 would simplify this.
- the lack of changeset descriptions is a bit irritating and makes
it much harder to understand what you are doing.
- try to keep patches that move code around separate from those
that change it in any other way, for better reviewing.
- in "sg: preparation for request sharing", you seem to inadvertently
change the size of "struct sg_extended_info", making it 4 bytes
longer by adding two members.
- You should never use IS_ERR_OR_NULL() in normal code, that
is just a sign of a bad API. Make each function have consistent
error behavior.
- The "#if 0 /* temporary to shorten big patch */" trick breaks
bisection, that is probably worse than the larger patch.
- The split access_ok()/__copy_from_user() has fallen out of
favor because it has caused too many bugs in the past, just
use the combined copy_from_user() instead.
- ktime_to_ns(ktime_get_with_offset(TK_OFFS_BOOT)) followed
by a 64-bit division won't work on 32-bit machines, use
ktime_get_boottime_ts64() instead.
> Other additions described in the that document are these new ioctls:
>- SG_IOSUBMITultimately to replace write(sg_fd, ...)
>- SG_IORECEIVE to replace read(sg_fd, ...)
>- SG_IOABORT abort SCSI cmd in progress; new functionality
>- SG_SET_GET_EXTENDED has associated struct sg_extended_info
>
> The first three take a pointer to a struct sg_io_hdr (v3 interface) or
> a struct sg_io_v4 object. Both objects start with a 32 bit integer:
> 'S' identifies the v3 interface while 'Q' identifies the v4 interface.
I think the magic character was a mistake in the original design,
just like versioned interfaces in general. If you are extending an
interface in an incompatible way, the normal way would be to
have separate command codes, like SG_IORECEIVE_V3
and SG_IORECEIVE_V4, if you absolutely have to maintain
compatiblity with the old interface (which I think you don't in
case of SG_IORECEIVE).
For SG_IO, I can see why you want to support both the v3
and v4 structures plus the compat-v3 version, but I'd try to keep
them as separate as possible, and do something like
static int sg_ctl_sg_io(struct file *filp, struct sg_device *sdp,
struct sg_fd *sfp,
void __user *p)
{
int ret;
ret = sg_io_v4(filp,
Re: [PATCH v3 12/26] compat_ioctl: move more drivers to compat_ptr_ioctl
On Thu, Apr 25, 2019 at 5:22 PM Mauro Carvalho Chehab wrote: > Em Tue, 16 Apr 2019 22:25:33 +0200 Arnd Bergmann escreveu: > > If I understand your patch description well, using compat_ptr_ioctl > only works if the driver is not for s390, right? No, the purpose of compat_ptr_ioctl() is to make sure it works everywhere including s390. Even on s390 it tends to work most of the time, but for correctness the upper bit of a 32-bit pointer needs to be cleared, as compat_ptr_ioctl does, in case some application passes a pointer with that bit set. [IIRC, in the instruction pointer, the high bit is set, in data references it is ignored but usually cleared, but it may be left on for IP-relative address generation] Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 12/26] compat_ioctl: move more drivers to compat_ptr_ioctl
On Thu, Apr 25, 2019 at 5:35 PM Al Viro wrote:
>
> On Thu, Apr 25, 2019 at 12:21:53PM -0300, Mauro Carvalho Chehab wrote:
>
> > If I understand your patch description well, using compat_ptr_ioctl
> > only works if the driver is not for s390, right?
>
> No; s390 is where "oh, just set ->compat_ioctl same as ->unlocked_ioctl
> and be done with that; compat_ptr() is a no-op anyway" breaks. IOW,
> s390 is the reason for having compat_ptr_ioctl() in the first place;
> that thing works on all biarch architectures, as long as all stuff
> handled by ->ioctl() takes pointer to arch-independent object as
> argument. IOW,
> argument ignored => OK
> any arithmetical type => no go, compat_ptr() would bugger it
> pointer to int => OK
> pointer to string => OK
> pointer to u64 => OK
> pointer to struct {u64 addr; char s[11];} => OK
To be extra pedantic, the 'struct {u64 addr; char s[11];} '
case is also broken on x86, because sizeof (obj) is smaller
on i386, even though the location of the members are
the same. i.e. you can copy_from_user() this, but not
copy_to_user(), which overwrites 4 bytes after the end of
the 20-byte user structure.
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 12/26] compat_ioctl: move more drivers to compat_ptr_ioctl
On Thu, Apr 25, 2019 at 11:25 PM Johannes Berg
wrote:
> On Thu, 2019-04-25 at 17:55 +0200, Arnd Bergmann wrote:
> > On Thu, Apr 25, 2019 at 5:35 PM Al Viro wrote:
> > >
> > > On Thu, Apr 25, 2019 at 12:21:53PM -0300, Mauro Carvalho Chehab wrote:
> > >
> > > > If I understand your patch description well, using compat_ptr_ioctl
> > > > only works if the driver is not for s390, right?
> > >
> > > No; s390 is where "oh, just set ->compat_ioctl same as ->unlocked_ioctl
> > > and be done with that; compat_ptr() is a no-op anyway" breaks. IOW,
> > > s390 is the reason for having compat_ptr_ioctl() in the first place;
> > > that thing works on all biarch architectures, as long as all stuff
> > > handled by ->ioctl() takes pointer to arch-independent object as
> > > argument. IOW,
> > > argument ignored => OK
> > > any arithmetical type => no go, compat_ptr() would bugger it
> > > pointer to int => OK
> > > pointer to string => OK
> > > pointer to u64 => OK
> > > pointer to struct {u64 addr; char s[11];} => OK
> >
> > To be extra pedantic, the 'struct {u64 addr; char s[11];} '
> > case is also broken on x86, because sizeof (obj) is smaller
> > on i386, even though the location of the members are
> > the same. i.e. you can copy_from_user() this
>
> Actually, you can't even do that because the struct might sit at the end
> of a page and then you'd erroneously fault in this case.
>
> We had this a while ago with struct ifreq, see commit 98406133dd and its
> parents.
Yes, you are right. Very rare to hit with real-life code, but easily
reproduced by intentionally hitting it and clearly a bug.
As the saying goes
| the difference between "always works" and "almost always works"
| is called data corruption
here the difference is an -EFAULT.
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[GIT PULL net-next, resend] isdn: deprecate non-mISDN drivers
[resending, rebased on top of today's net-next] The following changes since commit 7b3ed2a137b077bc0967352088b0adb6049eed20: Merge branch '100GbE' of git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue (2019-05-30 15:17:05 -0700) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git tags/isdn-removal for you to fetch changes up to 6d97985072dc270032dc7a08631080bfd6253e82: isdn: move capi drivers to staging (2019-05-31 11:17:41 +0200) isdn: deprecate non-mISDN drivers When isdn4linux came up in the context of another patch series, I remembered that we had discussed removing it a while ago. It turns out that the suggestion from Karsten Keil wa to remove I4L in 2018 after the last public ISDN networks are shut down. This has happened now (with a very small number of exceptions), so I guess it's time to try again. We currently have three ISDN stacks in the kernel: the original isdn4linux (with the hisax driver), the newer CAPI (with four drivers), and finally the mISDN stack (supporting roughly the same hardware as hisax). As far as I can tell, anyone using ISDN with mainline kernel drivers in the past few years uses mISDN, and this is typically used for voice-only PBX installations that don't require a public network. The older stacks support additional features for data networks, but those typically make no sense any more if there is no network to connect to. My proposal for this time is to kill off isdn4linux entirely, as it seems to have been unusable for quite a while. This code has been abandoned for many years and it does cause problems for treewide maintenance as it tends to do everything that we try to stop doing. Birger Harzenetter mentioned that is is still using i4l in order to make use of the 'divert' feature that is not part of mISDN, but has otherwise moved on to mISDN for normal operation, like apparently everyone else. CAPI in turn is not quite as obsolete, but two of the drivers (avm and hysdn) don't seem to be used at all, while another one (gigaset) will stop being maintained as Paul Bolle is no longer able to test it after the network gets shut down in September. All three are now moved into drivers/staging to let others speak up in case there are remaining users. This leaves Bluetooth CMTP as the only remaining user of CAPI, but Marcel Holtmann wishes to keep maintaining it. For the discussion on version 1, see [2] Unfortunately, Karsten Keil as the maintainer has not participated in the discussion. Arnd [1] https://patchwork.kernel.org/patch/8484861/#17900371 [2] https://listserv.isdn4linux.de/pipermail/isdn4linux/2019-April/thread.html ---- Arnd Bergmann (5): isdn: gigaset: remove i4l support isdn: remove hisax driver isdn: remove isdn4linux isdn: hdlc: move into mISDN isdn: move capi drivers to staging Documentation/isdn/HiSax.cert | 96 - Documentation/isdn/INTERFACE | 759 Documentation/isdn/INTERFACE.fax | 163 - Documentation/isdn/README | 599 Documentation/isdn/README.FAQ | 26 - Documentation/isdn/README.HiSax| 659 Documentation/isdn/README.audio| 138 - Documentation/isdn/README.concap | 259 -- Documentation/isdn/README.diversion| 127 - Documentation/isdn/README.fax | 45 - Documentation/isdn/README.gigaset | 36 +- Documentation/isdn/README.hfc-pci | 41 - Documentation/isdn/README.syncppp | 58 - Documentation/isdn/README.x25 | 184 - Documentation/isdn/syncPPP.FAQ | 224 -- Documentation/process/changes.rst | 16 +- MAINTAINERS| 22 +- drivers/isdn/Kconfig | 51 - drivers/isdn/Makefile |6 - drivers/isdn/capi/Kconfig | 29 +- drivers/isdn/capi/Makefile |2 + drivers/isdn/capi/capidrv.c| 2525 - drivers/isdn/capi/capidrv.h| 140 - drivers/isdn/divert/Makefile | 10 - drivers/isdn/divert/divert_init.c | 82 - drivers/isdn/divert/divert_procfs.c| 336 -- drivers/isdn/divert/isdn_divert.c | 846 - drivers/isdn/divert/isdn_divert.h | 132 - drivers/isdn/gigaset/i4l.c | 695 drivers/isdn/hardware/Kconfig |8 - drivers/isdn/hardware/Makefile |
Re: [net-next:master 391/455] drivers/staging/isdn/avm/b1.c:163:49: sparse: sparse: incorrect type in argument 2 (different address spaces)
On Mon, Jun 3, 2019 at 1:40 PM kbuild test robot wrote: > > Hi Arnd, > > First bad commit (maybe != root cause): Yep: > >> drivers/staging/isdn/avm/b1.c:163:49: sparse: sparse: incorrect type in > >> argument 2 (different address spaces) @@expected void const [noderef] > >> *from @@got const [noderef] *from @@ > >> drivers/staging/isdn/avm/b1.c:163:49: sparse:expected void const > >> [noderef] *from > >> drivers/staging/isdn/avm/b1.c:163:49: sparse:got unsigned char > >> *[assigned] dp I only moved the file, so the warnings are now for a different pathname. I'll leave it for the staging driver crowd to fix them up, or ignore them as the driver is on its way out of the kernel. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: iio: adt7316: use correct headers for gpio
When building without CONFIG_GPIOLIB, we get a compile-time failure:
drivers/staging/iio/addac/adt7316.c:947:3: error: implicit declaration of
function 'gpiod_set_value' [-Werror,-Wimplicit-function-declaration]
gpiod_set_value(chip->ldac_pin, 0);
^
drivers/staging/iio/addac/adt7316.c:947:3: note: did you mean 'gpio_set_value'?
include/linux/gpio.h:169:20: note: 'gpio_set_value' declared here
static inline void gpio_set_value(unsigned gpio, int value)
^
drivers/staging/iio/addac/adt7316.c:947:3: error: this function declaration is
not a prototype [-Werror,-Wstrict-prototypes]
gpiod_set_value(chip->ldac_pin, 0);
^
drivers/staging/iio/addac/adt7316.c:1805:13: error: implicit declaration of
function 'irqd_get_trigger_type' [-Werror,-Wimplicit-function-declaration]
irq_type = irqd_get_trigger_type(irq_get_irq_data(chip->bus.irq));
Include the correct headers that contain the declarations for these
functions.
Fixes: c63460c4298f ("Staging: iio: adt7316: Use device tree data to set
ldac_pin")
Signed-off-by: Arnd Bergmann
---
drivers/staging/iio/addac/adt7316.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/addac/adt7316.c
b/drivers/staging/iio/addac/adt7316.c
index 37ce563cb0e1..9cb3d0e42c38 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -6,7 +6,8 @@
*/
#include
-#include
+#include
+#include
#include
#include
#include
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] media: meson: include linux/kthread.h
Without this header, we get a compilation error in some configurations:
drivers/staging/media/meson/vdec/vdec.c: In function 'vdec_recycle_thread':
drivers/staging/media/meson/vdec/vdec.c:59:10: error: implicit declaration of
function 'kthread_should_stop' [-Werror=implicit-function-declaration]
Fixes: 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/meson/vdec/vdec.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/meson/vdec/vdec.c
b/drivers/staging/media/meson/vdec/vdec.c
index 0a1a04fd5d13..eb335a0f2bdd 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] lpfc: reduce stack size with CONFIG_GCC_PLUGIN_STRUCTLEAK_VERBOSE
The lpfc_debug_dump_all_queues() function repeatedly calls into
lpfc_debug_dump_qe(), which has a temporary 128 byte buffer.
This was fine before the introduction of CONFIG_GCC_PLUGIN_STRUCTLEAK_VERBOSE
because each instance could occupy the same stack slot. However,
now they each get their own copy, which leads to a huge increase
in stack usage as seen from the compiler warning:
drivers/scsi/lpfc/lpfc_debugfs.c: In function 'lpfc_debug_dump_all_queues':
drivers/scsi/lpfc/lpfc_debugfs.c:6474:1: error: the frame size of 1712 bytes is
larger than 100 bytes [-Werror=frame-larger-than=]
Avoid this by not marking lpfc_debug_dump_qe() as inline so the
compiler can choose to emit a static version of this function
when it's needed or otherwise silently drop it. As an added benefit,
not inlining multiple copies of this function means we save several
kilobytes of .text section, reducing the file size from 47kb to 43.
It is somewhat unusual to have a function that is static but not
inline in a header file, but this does not cause problems here
because it is only used by other inline functions. It would
however seem reasonable to move all the lpfc_debug_dump_* functions
into lpfc_debugfs.c and not mark them inline as a later cleanup.
Fixes: 81a56f6dcd20 ("gcc-plugins: structleak: Generalize to all variable
types")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/lpfc/lpfc_debugfs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.h b/drivers/scsi/lpfc/lpfc_debugfs.h
index 2322ddb085c0..34070874616d 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.h
+++ b/drivers/scsi/lpfc/lpfc_debugfs.h
@@ -330,7 +330,7 @@ enum {
* This function dumps an entry indexed by @idx from a queue specified by the
* queue descriptor @q.
**/
-static inline void
+static void
lpfc_debug_dump_qe(struct lpfc_queue *q, uint32_t idx)
{
char line_buf[LPFC_LBUF_SZ];
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] ipvs: reduce kernel stack usage
With the new CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL option, the stack
usage in the ipvs debug output grows because each instance of
IP_VS_DBG_BUF() now has its own buffer of 160 bytes that add up
rather than reusing the stack slots:
net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_sched_persist':
net/netfilter/ipvs/ip_vs_core.c:427:1: error: the frame size of 1052 bytes is
larger than 1024 bytes [-Werror=frame-larger-than=]
net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_new_conn_out':
net/netfilter/ipvs/ip_vs_core.c:1231:1: error: the frame size of 1048 bytes is
larger than 1024 bytes [-Werror=frame-larger-than=]
net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_out':
net/netfilter/ipvs/ip_vs_ftp.c:397:1: error: the frame size of 1104 bytes is
larger than 1024 bytes [-Werror=frame-larger-than=]
net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_in':
net/netfilter/ipvs/ip_vs_ftp.c:555:1: error: the frame size of 1200 bytes is
larger than 1024 bytes [-Werror=frame-larger-than=]
Since printk() already has a way to print IPv4/IPv6 addresses using
the %pIS format string, use that instead, combined with a macro that
creates a local sockaddr structure on the stack. These will still
add up, but the stack frames are now under 200 bytes.
Signed-off-by: Arnd Bergmann
---
I'm not sure this actually does what I think it does. Someone
needs to verify that we correctly print the addresses here.
I've also only added three files that caused the warning messages
to be reported. There are still a lot of other instances of
IP_VS_DBG_BUF() that could be converted the same way after the
basic idea is confirmed.
---
include/net/ip_vs.h | 71 +++--
net/netfilter/ipvs/ip_vs_core.c | 44 ++--
net/netfilter/ipvs/ip_vs_ftp.c | 20 +-
3 files changed, 72 insertions(+), 63 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 3759167f91f5..3dfbeef67be6 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -227,6 +227,16 @@ static inline const char *ip_vs_dbg_addr(int af, char
*buf, size_t buf_len,
sizeof(ip_vs_dbg_buf), addr, \
&ip_vs_dbg_idx)
+#define IP_VS_DBG_SOCKADDR4(fam, addr, port) \
+ (struct sockaddr*)&(struct sockaddr_in) \
+ { .sin_family = (fam), .sin_addr = (addr)->in, .sin_port = (port) }
+#define IP_VS_DBG_SOCKADDR6(fam, addr, port) \
+ (struct sockaddr*)&(struct sockaddr_in6) \
+ { .sin6_family = (fam), .sin6_addr = (addr)->in6, .sin6_port = (port) }
+#define IP_VS_DBG_SOCKADDR(fam, addr, port) (fam == AF_INET ? \
+ IP_VS_DBG_SOCKADDR4(fam, addr, port) : \
+ IP_VS_DBG_SOCKADDR6(fam, addr, port))
+
#define IP_VS_DBG(level, msg, ...) \
do {\
if (level <= ip_vs_get_debug_level()) \
@@ -251,6 +261,7 @@ static inline const char *ip_vs_dbg_addr(int af, char *buf,
size_t buf_len,
#else /* NO DEBUGGING at ALL */
#define IP_VS_DBG_BUF(level, msg...) do {} while (0)
#define IP_VS_ERR_BUF(msg...) do {} while (0)
+#define IP_VS_DBG_SOCKADDR(fam, addr, port) NULL
#define IP_VS_DBG(level, msg...) do {} while (0)
#define IP_VS_DBG_RL(msg...) do {} while (0)
#define IP_VS_DBG_PKT(level, af, pp, skb, ofs, msg)do {} while (0)
@@ -1244,31 +1255,31 @@ static inline void ip_vs_control_del(struct ip_vs_conn
*cp)
{
struct ip_vs_conn *ctl_cp = cp->control;
if (!ctl_cp) {
- IP_VS_ERR_BUF("request control DEL for uncontrolled: "
- "%s:%d to %s:%d\n",
- IP_VS_DBG_ADDR(cp->af, &cp->caddr),
- ntohs(cp->cport),
- IP_VS_DBG_ADDR(cp->af, &cp->vaddr),
- ntohs(cp->vport));
+ pr_err("request control DEL for uncontrolled: "
+ "%pISp to %pISp\n",
+ IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr,
+ ntohs(cp->cport)),
+ IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr,
+ ntohs(cp->vport)));
return;
}
- IP_VS_DBG_BUF(7, "DELeting control for: "
- "cp.dst=%s:%d ctl_cp.dst=%s:%d\n",
- IP_VS_DBG_ADDR(cp->af, &cp->caddr),
- ntohs(cp->cport),
- IP_VS_DBG_ADDR(cp->af, &ctl_cp->caddr),
- ntohs(ctl_cp->cport));
+ IP_VS_DBG(7, "DELeting cont
[PATCH 1/4] [v2] structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK
The combination of KASAN_STACK and GCC_PLUGIN_STRUCTLEAK_BYREF
leads to much larger kernel stack usage, as seen from the warnings
about functions that now exceed the 2048 byte limit:
drivers/media/i2c/tvp5150.c:253:1: error: the frame size of 3936 bytes is
larger than 2048 bytes
drivers/media/tuners/r820t.c:1327:1: error: the frame size of 2816 bytes is
larger than 2048 bytes
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:16552:1: error:
the frame size of 3144 bytes is larger than 2048 bytes
[-Werror=frame-larger-than=]
fs/ocfs2/aops.c:1892:1: error: the frame size of 2088 bytes is larger than 2048
bytes
fs/ocfs2/dlm/dlmrecovery.c:737:1: error: the frame size of 2088 bytes is larger
than 2048 bytes
fs/ocfs2/namei.c:1677:1: error: the frame size of 2584 bytes is larger than
2048 bytes
fs/ocfs2/super.c:1186:1: error: the frame size of 2640 bytes is larger than
2048 bytes
fs/ocfs2/xattr.c:3678:1: error: the frame size of 2176 bytes is larger than
2048 bytes
net/bluetooth/l2cap_core.c:7056:1: error: the frame size of 2144 bytes is
larger than 2048 bytes [-Werror=frame-larger-than=]
net/bluetooth/l2cap_core.c: In function 'l2cap_recv_frame':
net/bridge/br_netlink.c:1505:1: error: the frame size of 2448 bytes is larger
than 2048 bytes
net/ieee802154/nl802154.c:548:1: error: the frame size of 2232 bytes is larger
than 2048 bytes
net/wireless/nl80211.c:1726:1: error: the frame size of 2224 bytes is larger
than 2048 bytes
net/wireless/nl80211.c:2357:1: error: the frame size of 4584 bytes is larger
than 2048 bytes
net/wireless/nl80211.c:5108:1: error: the frame size of 2760 bytes is larger
than 2048 bytes
net/wireless/nl80211.c:6472:1: error: the frame size of 2112 bytes is larger
than 2048 bytes
The structleak plugin was previously disabled for CONFIG_COMPILE_TEST,
but meant we missed some bugs, so this time we should address them.
The frame size warnings are distracting, and risking a kernel stack
overflow is generally not beneficial to performance, so it may be best
to disallow that particular combination. This can be done by turning
off either one. I picked the dependency in GCC_PLUGIN_STRUCTLEAK_BYREF
and GCC_PLUGIN_STRUCTLEAK_BYREF_ALL, as this option is designed to
make uninitialized stack usage less harmful when enabled on its own,
but it also prevents KASAN from detecting those cases in which it was
in fact needed.
KASAN_STACK is currently implied by KASAN on gcc, but could be made a
user selectable option if we want to allow combining (non-stack) KASAN
with GCC_PLUGIN_STRUCTLEAK_BYREF.
Note that it would be possible to specifically address the files that
print the warning, but presumably the overall stack usage is still
significantly higher than in other configurations, so this would not
address the full problem.
I could not test this with CONFIG_INIT_STACK_ALL, which may or may not
suffer from a similar problem.
Fixes: 81a56f6dcd20 ("gcc-plugins: structleak: Generalize to all variable
types")
Signed-off-by: Arnd Bergmann
---
[v2] do it for both GCC_PLUGIN_STRUCTLEAK_BYREF and
GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
---
security/Kconfig.hardening | 7 +++
1 file changed, 7 insertions(+)
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index a1ffe2eb4d5f..af4c979b38ee 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -61,6 +61,7 @@ choice
config GCC_PLUGIN_STRUCTLEAK_BYREF
bool "zero-init structs passed by reference (strong)"
depends on GCC_PLUGINS
+ depends on !(KASAN && KASAN_STACK=1)
select GCC_PLUGIN_STRUCTLEAK
help
Zero-initialize any structures on the stack that may
@@ -70,9 +71,15 @@ choice
exposures, like CVE-2017-1000410:
https://git.kernel.org/linus/06e7e776ca4d3654
+ As a side-effect, this keeps a lot of variables on the
+ stack that can otherwise be optimized out, so combining
+ this with CONFIG_KASAN_STACK can lead to a stack overflow
+ and is disallowed.
+
config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
bool "zero-init anything passed by reference (very strong)"
depends on GCC_PLUGINS
+ depends on !(KASAN && KASAN_STACK=1)
select GCC_PLUGIN_STRUCTLEAK
help
Zero-initialize any stack variables that may be passed
--
2.20.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] staging: rtl8712: reduce stack usage, again
An earlier patch I sent reduced the stack usage enough to get
below the warning limit, and I could show this was safe, but with
GCC_PLUGIN_STRUCTLEAK_BYREF_ALL, it gets worse again because large stack
variables in the same function no longer overlap:
drivers/staging/rtl8712/rtl871x_ioctl_linux.c: In function
'translate_scan.isra.2':
drivers/staging/rtl8712/rtl871x_ioctl_linux.c:322:1: error: the frame size of
1200 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Split out the largest two blocks in the affected function into two
separate functions and mark those noinline_for_stack.
Fixes: 8c5af16f7953 ("staging: rtl8712: reduce stack usage")
Fixes: 81a56f6dcd20 ("gcc-plugins: structleak: Generalize to all variable
types")
Signed-off-by: Arnd Bergmann
---
drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 157 ++
1 file changed, 88 insertions(+), 69 deletions(-)
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index a224797cd993..fdc1df99d852 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -124,10 +124,91 @@ static inline void handle_group_key(struct ieee_param
*param,
}
}
-static noinline_for_stack char *translate_scan(struct _adapter *padapter,
- struct iw_request_info *info,
- struct wlan_network *pnetwork,
- char *start, char *stop)
+static noinline_for_stack char *translate_scan_wpa(struct iw_request_info
*info,
+ struct wlan_network
*pnetwork,
+ struct iw_event *iwe,
+ char *start, char *stop)
+{
+ /* parsing WPA/WPA2 IE */
+ u8 buf[MAX_WPA_IE_LEN];
+ u8 wpa_ie[255], rsn_ie[255];
+ u16 wpa_len = 0, rsn_len = 0;
+ int n, i;
+
+ r8712_get_sec_ie(pnetwork->network.IEs,
+pnetwork->network.IELength, rsn_ie, &rsn_len,
+wpa_ie, &wpa_len);
+ if (wpa_len > 0) {
+ memset(buf, 0, MAX_WPA_IE_LEN);
+ n = sprintf(buf, "wpa_ie=");
+ for (i = 0; i < wpa_len; i++) {
+ n += snprintf(buf + n, MAX_WPA_IE_LEN - n,
+ "%02x", wpa_ie[i]);
+ if (n >= MAX_WPA_IE_LEN)
+ break;
+ }
+ memset(iwe, 0, sizeof(*iwe));
+ iwe->cmd = IWEVCUSTOM;
+ iwe->u.data.length = (u16)strlen(buf);
+ start = iwe_stream_add_point(info, start, stop,
+ iwe, buf);
+ memset(iwe, 0, sizeof(*iwe));
+ iwe->cmd = IWEVGENIE;
+ iwe->u.data.length = (u16)wpa_len;
+ start = iwe_stream_add_point(info, start, stop,
+ iwe, wpa_ie);
+ }
+ if (rsn_len > 0) {
+ memset(buf, 0, MAX_WPA_IE_LEN);
+ n = sprintf(buf, "rsn_ie=");
+ for (i = 0; i < rsn_len; i++) {
+ n += snprintf(buf + n, MAX_WPA_IE_LEN - n,
+ "%02x", rsn_ie[i]);
+ if (n >= MAX_WPA_IE_LEN)
+ break;
+ }
+ memset(iwe, 0, sizeof(*iwe));
+ iwe->cmd = IWEVCUSTOM;
+ iwe->u.data.length = strlen(buf);
+ start = iwe_stream_add_point(info, start, stop,
+ iwe, buf);
+ memset(iwe, 0, sizeof(*iwe));
+ iwe->cmd = IWEVGENIE;
+ iwe->u.data.length = rsn_len;
+ start = iwe_stream_add_point(info, start, stop, iwe,
+ rsn_ie);
+ }
+
+ return start;
+}
+
+static noinline_for_stack char *translate_scan_wps(struct iw_request_info
*info,
+ struct wlan_network
*pnetwork,
+ struct iw_event *iwe,
+ char *start, char *stop)
+{
+ /* parsing WPS IE */
+ u8 wps_ie[512];
+ uint wps_ielen;
+
+ if (r8712_get_wps_ie(pnetwork->network.IEs,
+ pnetwork->network.IELength,
+ wps_ie, &wps_ielen)) {
+ if (wps_ielen > 2) {
+ iwe->cmd = IWEVGENIE;
+ iwe->u.data.length = (u16)wps_ielen;
+ start = iwe_stream_add_point(info, start, stop,
+ iwe, wps_ie);
+ }
+ }
+
+ return start;
+}
+
+static char *translate_sc
[PATCH] staging: media/davinci_vpfe: fix pinmux setup compilation
The dm365_isif staging driver uses an odd method for configuring its
pin muxing by calling directly into low-level davinci platform specific
code, even when being compile-tested for other platforms.
As we want davinci to be part of a multi-platform kernel, this will
cause a build failure when those headers are no longer exported even
for davinci:
drivers/staging/media/davinci_vpfe/dm365_isif.c: In function 'vpfe_isif_init':
drivers/staging/media/davinci_vpfe/dm365_isif.c:2031:2: error: implicit
declaration of function 'davinci_cfg_reg'; did you mean 'omap_cfg_reg'?
[-Werror=implicit-function-declaration]
davinci_cfg_reg(DM365_VIN_CAM_WEN);
^~~
omap_cfg_reg
drivers/staging/media/davinci_vpfe/dm365_isif.c:2031:18: error:
'DM365_VIN_CAM_WEN' undeclared (first use in this function); did you mean
'DM365_ISIF_MAX_CLDC'?
davinci_cfg_reg(DM365_VIN_CAM_WEN);
^
Digging further, it seems that the platform data structures defined
in drivers/staging/media/davinci_vpfe/vpfe.h are an incompatible
version of the same structures in include/media/davinci/vpfe_capture.h,
which is the version that is used by the platform code, so the
combination that exists in the mainline kernel cannot be used.
The platform code already has an abstraction for the pinmux,
in the form of the dm365_isif_setup_pinmux() helper. If we want
to ever get to use the staging driver again, this needs to be
read from the platform data passed to this driver, or rewritten
to use the pinmux framework.
For the moment, pretend we pass the helper function in the
staging platform driver to get it to build cleanly. I could
not figure out how the staging driver relates to the code
in drivers/media/platform/davinci/, some clarification on that
would be helpful to decide what the long-term plan on this
should be to either remove the staging driver as obsolete or
integrate it with the rest in a way that actually works.
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/davinci_vpfe/Makefile | 5 -
drivers/staging/media/davinci_vpfe/dm365_isif.c | 8 +++-
drivers/staging/media/davinci_vpfe/dm365_isif.h | 2 --
drivers/staging/media/davinci_vpfe/vpfe.h | 2 ++
4 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/media/davinci_vpfe/Makefile
b/drivers/staging/media/davinci_vpfe/Makefile
index 0ae8c5014f74..3b93e0583940 100644
--- a/drivers/staging/media/davinci_vpfe/Makefile
+++ b/drivers/staging/media/davinci_vpfe/Makefile
@@ -4,8 +4,3 @@ obj-$(CONFIG_VIDEO_DM365_VPFE) += davinci-vfpe.o
davinci-vfpe-objs := \
dm365_isif.o dm365_ipipe_hw.o dm365_ipipe.o \
dm365_resizer.o dm365_ipipeif.o vpfe_mc_capture.o vpfe_video.o
-
-# Allow building it with COMPILE_TEST on other archs
-ifndef CONFIG_ARCH_DAVINCI
-ccflags-y += -I $(srctree)/arch/arm/mach-davinci/include/
-endif
diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.c
b/drivers/staging/media/davinci_vpfe/dm365_isif.c
index 05a997f7aa5d..5cfd52ea3ba7 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_isif.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_isif.c
@@ -17,6 +17,7 @@
*/
#include
+#include "vpfe.h"
#include "dm365_isif.h"
#include "vpfe_mc_capture.h"
@@ -1983,6 +1984,7 @@ int vpfe_isif_init(struct vpfe_isif_device *isif, struct
platform_device *pdev)
struct v4l2_subdev *sd = &isif->subdev;
struct media_pad *pads = &isif->pads[0];
struct media_entity *me = &sd->entity;
+ struct vpfe_config *cfg = pdev->dev.platform_data;
static resource_size_t res_len;
struct resource *res;
void __iomem *addr;
@@ -2023,11 +2025,7 @@ int vpfe_isif_init(struct vpfe_isif_device *isif, struct
platform_device *pdev)
}
i++;
}
- davinci_cfg_reg(DM365_VIN_CAM_WEN);
- davinci_cfg_reg(DM365_VIN_CAM_VD);
- davinci_cfg_reg(DM365_VIN_CAM_HD);
- davinci_cfg_reg(DM365_VIN_YIN4_7_EN);
- davinci_cfg_reg(DM365_VIN_YIN0_3_EN);
+ cfg->isif_setup_pinmux();
/* queue ops */
isif->video_out.ops = &isif_video_ops;
diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.h
b/drivers/staging/media/davinci_vpfe/dm365_isif.h
index 0e1fe472fb2b..82eeba9c24c2 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_isif.h
+++ b/drivers/staging/media/davinci_vpfe/dm365_isif.h
@@ -21,8 +21,6 @@
#include
-#include
-
#include
#include
#include
diff --git a/drivers/staging/media/davinci_vpfe/vpfe.h
b/drivers/staging/media/davinci_vpfe/vpfe.h
index 1f8e011fc162..54ef6720ceeb 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe.h
+++ b/drivers/staging/media/davinci_vpfe/vpfe.h
@@ -74,6 +74,8 @@ struct vpfe_config {
char *card_name;
/* setup function for the input path */
int (*setup_input)(enum vpfe_subdev_id id);
+
Re: [PATCH 4/4] ipvs: reduce kernel stack usage
On Sun, Jun 30, 2019 at 10:37 PM Julian Anastasov wrote:
> On Fri, 28 Jun 2019, Arnd Bergmann wrote:
> > struct ip_vs_conn *ctl_cp = cp->control;
> > if (!ctl_cp) {
> > - IP_VS_ERR_BUF("request control DEL for uncontrolled: "
> > - "%s:%d to %s:%d\n",
> > - IP_VS_DBG_ADDR(cp->af, &cp->caddr),
> > - ntohs(cp->cport),
> > - IP_VS_DBG_ADDR(cp->af, &cp->vaddr),
> > - ntohs(cp->vport));
> > + pr_err("request control DEL for uncontrolled: "
> > +"%pISp to %pISp\n",
(replying a bit late)
> ip_vs_dbg_addr() used compact form (%pI6c), so it would be
> better to use %pISc and %pISpc everywhere in IPVS...
done
> Also, note that before now port was printed with %d and
> ntohs() was used, now port should be in network order, so:
>
> - ntohs() should be removed
done
> - htons() should be added, if missing. At first look, this case
> is not present in IPVS, we have only ntohs() usage
I found one case in ip_vs_ftp_in() that needs it in order to subtract one:
IP_VS_DBG(7, "protocol %s %pISpc %pISpc\n",
ip_vs_proto_name(ipvsh->protocol),
- IP_VS_DBG_SOCKADDR(cp->af, &to, ntohs(port)),
+ IP_VS_DBG_SOCKADDR(cp->af, &to, port),
IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr,
- ntohs(cp->vport)-1));
+htons(ntohs(cp->vport)-1)));
Thanks for the review, I'll send a new patch after some more
build testing on the new version.
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] ipvs: reduce kernel stack usage
On Fri, Jun 28, 2019 at 9:59 PM Willem de Bruijn
wrote:
> On Fri, Jun 28, 2019 at 8:40 AM Arnd Bergmann wrote:
> >
> > With the new CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL option, the stack
> > usage in the ipvs debug output grows because each instance of
> > IP_VS_DBG_BUF() now has its own buffer of 160 bytes that add up
> > rather than reusing the stack slots:
> >
> > net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_sched_persist':
> > net/netfilter/ipvs/ip_vs_core.c:427:1: error: the frame size of 1052 bytes
> > is larger than 1024 bytes [-Werror=frame-larger-than=]
> > net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_new_conn_out':
> > net/netfilter/ipvs/ip_vs_core.c:1231:1: error: the frame size of 1048 bytes
> > is larger than 1024 bytes [-Werror=frame-larger-than=]
> > net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_out':
> > net/netfilter/ipvs/ip_vs_ftp.c:397:1: error: the frame size of 1104 bytes
> > is larger than 1024 bytes [-Werror=frame-larger-than=]
> > net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_in':
> > net/netfilter/ipvs/ip_vs_ftp.c:555:1: error: the frame size of 1200 bytes
> > is larger than 1024 bytes [-Werror=frame-larger-than=]
> >
> > Since printk() already has a way to print IPv4/IPv6 addresses using
> > the %pIS format string, use that instead,
>
> since these are sockaddr_in and sockaddr_in6, should that have the 'n'
> specifier to denote network byteorder?
I double-checked the implementation, and don't see that make any difference,
as 'n' is the default.
> > include/net/ip_vs.h | 71 +++--
> > net/netfilter/ipvs/ip_vs_core.c | 44 ++--
> > net/netfilter/ipvs/ip_vs_ftp.c | 20 +-
> > 3 files changed, 72 insertions(+), 63 deletions(-)
> >
> > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> > index 3759167f91f5..3dfbeef67be6 100644
> > --- a/include/net/ip_vs.h
> > +++ b/include/net/ip_vs.h
> > @@ -227,6 +227,16 @@ static inline const char *ip_vs_dbg_addr(int af, char
> > *buf, size_t buf_len,
> >sizeof(ip_vs_dbg_buf), addr, \
> >&ip_vs_dbg_idx)
> >
> > +#define IP_VS_DBG_SOCKADDR4(fam, addr, port) \
> > + (struct sockaddr*)&(struct sockaddr_in) \
> > + { .sin_family = (fam), .sin_addr = (addr)->in, .sin_port = (port) }
>
> might as well set .sin_family = AF_INET here and AF_INET6 below?
That would work just same, but I don't see any advantage. As the family
and port members are the same between sockaddr_in and sockaddr_in6,
the compiler can decide to set these regardless to the argument values
regardless of the condition.
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 12/29] compat_ioctl: move drivers to compat_ptr_ioctl
Each of these drivers has a copy of the same trivial helper function to
convert the pointer argument and then call the native ioctl handler.
We now have a generic implementation of that, so use it.
Acked-by: Greg Kroah-Hartman
Acked-by: Michael S. Tsirkin
Reviewed-by: Jarkko Sakkinen
Reviewed-by: Jason Gunthorpe
Reviewed-by: Jiri Kosina
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Arnd Bergmann
---
drivers/char/ppdev.c | 12 +-
drivers/char/tpm/tpm_vtpm_proxy.c | 12 +-
drivers/firewire/core-cdev.c | 12 +-
drivers/hid/usbhid/hiddev.c | 11 +
drivers/hwtracing/stm/core.c | 12 +-
drivers/misc/mei/main.c | 22 +
drivers/mtd/ubi/cdev.c| 36 +++-
drivers/net/tap.c | 12 +-
drivers/staging/pi433/pi433_if.c | 12 +-
drivers/usb/core/devio.c | 16 +
drivers/vfio/vfio.c | 39 +++
drivers/vhost/net.c | 12 +-
drivers/vhost/scsi.c | 12 +-
drivers/vhost/test.c | 12 +-
drivers/vhost/vsock.c | 12 +-
fs/ceph/dir.c | 2 +-
fs/ceph/file.c| 2 +-
fs/ceph/super.h | 9 ---
fs/fat/file.c | 13 +--
19 files changed, 22 insertions(+), 248 deletions(-)
diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index f0a8adca1eee..c4d5cc4a1d3e 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -670,14 +670,6 @@ static long pp_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
return ret;
}
-#ifdef CONFIG_COMPAT
-static long pp_compat_ioctl(struct file *file, unsigned int cmd,
- unsigned long arg)
-{
- return pp_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
static int pp_open(struct inode *inode, struct file *file)
{
unsigned int minor = iminor(inode);
@@ -786,9 +778,7 @@ static const struct file_operations pp_fops = {
.write = pp_write,
.poll = pp_poll,
.unlocked_ioctl = pp_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = pp_compat_ioctl,
-#endif
+ .compat_ioctl = compat_ptr_ioctl,
.open = pp_open,
.release= pp_release,
};
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c
b/drivers/char/tpm/tpm_vtpm_proxy.c
index 2f6e087ec496..91c772e38bb5 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -670,20 +670,10 @@ static long vtpmx_fops_ioctl(struct file *f, unsigned int
ioctl,
}
}
-#ifdef CONFIG_COMPAT
-static long vtpmx_fops_compat_ioctl(struct file *f, unsigned int ioctl,
- unsigned long arg)
-{
- return vtpmx_fops_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
-}
-#endif
-
static const struct file_operations vtpmx_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = vtpmx_fops_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = vtpmx_fops_compat_ioctl,
-#endif
+ .compat_ioctl = compat_ptr_ioctl,
.llseek = noop_llseek,
};
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 1da7ba18d399..c777088f5828 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1646,14 +1646,6 @@ static long fw_device_op_ioctl(struct file *file,
return dispatch_ioctl(file->private_data, cmd, (void __user *)arg);
}
-#ifdef CONFIG_COMPAT
-static long fw_device_op_compat_ioctl(struct file *file,
- unsigned int cmd, unsigned long arg)
-{
- return dispatch_ioctl(file->private_data, cmd, compat_ptr(arg));
-}
-#endif
-
static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma)
{
struct client *client = file->private_data;
@@ -1795,7 +1787,5 @@ const struct file_operations fw_device_ops = {
.mmap = fw_device_op_mmap,
.release= fw_device_op_release,
.poll = fw_device_op_poll,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = fw_device_op_compat_ioctl,
-#endif
+ .compat_ioctl = compat_ptr_ioctl,
};
diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index 55b72573066b..70009bd76ac1 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -842,13 +842,6 @@ static long hiddev_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
return r;
}
-#ifdef CONFIG_COMPAT
-static long hiddev_compat_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
-{
- return hiddev_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
static const struct file_operations hiddev_fops = {
.owner =THIS_MODULE,
.read = hiddev_read,
@@ -858,9 +851,7 @@ static con
[PATCH] staging: unisys/visorbus: add __init/__exit annotations
gcc-4.6 causes a harmless warning about the init function:
WARNING: vmlinux.o(.text+0xed62c2): Section mismatch in reference from the
function init_unisys() to the function .init.text:visorutil_spar_detect()
The function init_unisys() references
the function __init visorutil_spar_detect().
This is often because init_unisys lacks a __init
annotation or the annotation of visorutil_spar_detect is wrong.
It appears that newer versions inline visorutil_spar_detect(),
end up with an empty __init section. This marks the module
entry points as __init and __exit respectively, which avoids
the warning and slightly reduces the runtime code size.
Signed-off-by: Arnd Bergmann
---
drivers/staging/unisys/visorbus/visorchipset.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c
b/drivers/staging/unisys/visorbus/visorchipset.c
index 74cce4f1a7bd..27ecf6fb49fd 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -1826,7 +1826,7 @@ static __init int visorutil_spar_detect(void)
return 0;
}
-static int init_unisys(void)
+static int __init init_unisys(void)
{
int result;
@@ -1841,7 +1841,7 @@ static int init_unisys(void)
return 0;
};
-static void exit_unisys(void)
+static void __exit exit_unisys(void)
{
acpi_bus_unregister_driver(&unisys_acpi_driver);
}
--
2.9.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: fix type mismatch warning
On Mon, Sep 18, 2017 at 4:02 PM, Greg Kroah-Hartman wrote: >> However, there is another problem with the Kconfig option: turning >> it on or off creates two incompatible ABI versions, a kernel that >> has this enabled cannot run user space that was built without it >> or vice versa. A better solution might be to leave the option hidden >> until the binder code is fixed to deal with both ABI versions. > > I don't know if that is ever going to be fixed, as it's not an issue > that you will ever see "in the wild" from what I can tell... Probably not on a native Android device or even a Chromebook that ships a binder user space together with a kernel, but what about people using "anbox" or similar projects that allow you to run Android apps in a container? It seems like a legitimate use case of the binder modules, but now there is a kernel Kconfig option that has to match a user space binary. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: fix type mismatch warning
On Mon, Sep 18, 2017 at 6:23 PM, Christoph Hellwig wrote: > On Mon, Sep 18, 2017 at 05:53:47PM +0200, Greg Kroah-Hartman wrote: >> > It seems like a legitimate use case of the binder modules, but >> > now there is a kernel Kconfig option that has to match a user >> > space binary. >> >> So, should we revert that? >> >> I don't really know what to suggest here, sorry... > > Keep it as is for now, and encourage people to fix it. But we should > reject any new patch that makes this worse or adds additional ioctls > that don't follow our normal model. I would argue we should leave it as non-configurable, but make a conscious decision which ABI to use for 4.14, as that is going to be in a the next generation of Android devices: a) leave the BINDER_CURRENT_PROTOCOL_VERSION==7 interface supported in all 32-bit builds forever, remaining compatible the ABI that was supported in mainline Linux in all versions until v4.13. The version 7 interface is incompatible between 32-bit and 64-bit user space, and there is no compat_ioctl implementation for it, so someone should fix it by implementing a proper compat_ioctl function. The current Kconfig comment says that v7 of the ABI is also incompatible with Android 4.5 and later user space. Can someone confirm that? The code looks like it's written under the assumption that user space would dynamically pick between v7 and v8 based on the return value of ioctl(..., BINDER_VERSION). We could also add support for switching the ABI dynamically in the kernel module based on either a module parameter or an ioctl that a future version of the binder user space can call, to add optional support for the v8 ABI on 64-bit. b) Treat the fact that we implemented the v7 binder ABI as a bug, since real Android uses v8, removing all traces of the v7 code from the kernel, and requiring users of Android 4.4 and earlier to upgrade their binder to a version that runs on modern kernels. We can probably get away with that because - Neither arm64 nor x86_64 are affected by this change. - "anbox" would normally only be used on x86_64, but not i386 or arm32 - we probably already lost support for Android 4.4 and earlier due to other kernel changes - any arm32 user space that people actually try to run with a mainline kernel should already support the v8 ABI, and might not support the v7 ABI at all. - we don't support v1 through v6 of the interface either. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: fix type mismatch warning
On Wed, Sep 20, 2017 at 11:08 AM, Martijn Coenen wrote: > On Mon, Sep 18, 2017 at 9:49 PM, Arnd Bergmann wrote: >> The current Kconfig comment says that v7 of the ABI is also >> incompatible with Android 4.5 and later user space. Can someone >> confirm that? > > That is not actually true - v7 does work with all versions of Android > (up to and including Oreo). In fact, we've polled some vendors, and > all vendors with a 32-bit SoC are using the v7 interface, even on > recent Android versions. Ah, good to know, thanks for providing that data point! >> The code looks like it's written under the assumption >> that user space would dynamically pick between v7 and v8 based >> on the return value of ioctl(..., BINDER_VERSION). > > It's a build-time configuration option in Android userspace. Ok. >> b) Treat the fact that we implemented the v7 binder ABI as a bug, >> since real Android uses v8, removing all traces of the v7 code from >> the kernel, and requiring users of Android 4.4 and earlier to upgrade >> their binder to a version that runs on modern kernels. > > This would be my proposal as well. In fact, we have already planned to > officially remove support for the v7 interface in Android P, and > require all devices using Android P to use the 64-bit v8 interface > (regardless of whether the machine is 32-bit or 64-bit). The one > caveat is that for stable/longterm, I think we want to leave the > config option, but perhaps flip the default (to v8). The main reason > is that some vendors may have existing devices on the v7 interface, > and maybe they're just pushing security updates. We don't want to > force them to switch to 64-bit just by pulling in the latest stable. > Does that sound reasonable? I see multiple problems with that: - On stable mainline kernels (unlike android-common), the v8 interface has never been available as a build option, and making it user-selectable will required additional patches to make it actually build on 32-bit ARM. This is fixable if Greg is willing to take the backports into stable kernels - Having a compile-time option to pick between two incompatible ABIs is a really bad idea, we don't do that anywhere else in the kernel except for things like the choice between big-endian and little-endian kernels that can't be done otherwise. If we want to keep supporting both ABIs going forward, I think it needs to be a runtime option instead, to allow users to migrate between kernel versions. - Since you say there are existing users of recent 32-bit Android including Oreo, I also think that removing support for the v7 ABI is no longer an option. I only made that suggestion under the assumption that there is no user space requiring that. Linux has no real way of "deprecating" an existing ABI, either it is currently used and has to stay around, or it is not used at all and can be removed without anybody noticing. If we end up doing a runtime switch between the ABIs instead, I see two ways of doing that: The easiest implementation would make it a module parameter: Since there is only one instance of the binder in any given system, and presumably all processes talking to it have to use the same ABI, this should be sufficient. The main downside is that it prevents us from having incompatible versions per namespace if we ever get a containerized version of binder. The correct way to do it would be to unify the two versions of the ABI so they can be used interchangeably: any 32-bit process would start out with the v7 interface and a 64-bit process would start out with the v8 interface, and any new version of the 32-bit binder user space would issue a special ioctl to switch to the v8 version. If we ever get a v9 version of the ABI, that would then add another set of ioctl commands with different numbers that don't conflict with the v8 interface but are implemented by the same kernel module. Actually implementing the combined v7/v8 ioctl stuff is of course nontrivial. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: fix type mismatch warning
On Wed, Sep 20, 2017 at 12:24 PM, Martijn Coenen wrote: > On Wed, Sep 20, 2017 at 11:58 AM, Arnd Bergmann wrote: >> >> - Since you say there are existing users of recent 32-bit Android >> including Oreo, I also think that removing support for the v7 ABI >> is no longer an option. I only made that suggestion under the >> assumption that there is no user space requiring that. Linux >> has no real way of "deprecating" an existing ABI, either it is >> currently used and has to stay around, or it is not used at all >> and can be removed without anybody noticing. > > I don't know of any Android devices shipping with 4.14 - we don't even > have a common tree for 4.14 (4.9 is the latest). So I don't think > we're hurting anyone in the Android ecosystem if we remove v7 from > 4.14. From what I know, it's extremely rare for an Android device to > change their kernel version after a device ships, but even if someone > hypothetically did update their Android device to use 4.14, they can > pretty easily switch the build-time config option. I understand that > this is against Linux' philosophy around not breaking userspace, I > just want to point out that I think from an Android POV, removing v7 > from 4.14 is not an issue. I'm not sure if that is good enough. I'm not really worried about shipping Android products, for those there is no big problem using the compile-time option as they build everything together. The case that gets interesting is a any kind of user that wants to run an Android application on a regular Linux box without using virtual machines or emulation, e.g. a an app developer, or a user that wants to run some Android app on their desktop using anbox. This obviously requires enabling the module, but it should not require enabling an option to support one version of the user space while breaking another version. >> If we end up doing a runtime switch between the ABIs instead, >> I see two ways of doing that: >> >> The easiest implementation would make it a module parameter: >> Since there is only one instance of the binder in any given >> system, and presumably all processes talking to it have to use >> the same ABI, this should be sufficient. The main downside is >> that it prevents us from having incompatible versions per >> namespace if we ever get a containerized version of binder. > > Namespace support for binder is currently being investigated, but I'm > not sure we'd ever have a system where we want to mix v7 and v8. There > really is no good reason to use v7 anymore (even on 32-bit mahcines), > we just should have removed it from our userspace a lot earlier. The only possible reason for v7 is backwards compatibility. I agree we don't need a single machine (or container) to support a mix of v7 and v8 applications, but I can see someone using a v7 based chroot user space today to keep running that in a container in the future while using another container for an updated image. This is clearly a corner case that we could decide to ignore, it just feels like a bit of a hack. >> The correct way to do it would be to unify the two versions of >> the ABI so they can be used interchangeably: any 32-bit >> process would start out with the v7 interface and a 64-bit >> process would start out with the v8 interface > > This wouldn't work with the current implementation - if a 32-bit and > 64-bit process communicate, then userspace would make wrong > assumptions about the sizes of transactions. Or is this what you're > proposing to fix? The scenario I had in mind is like this: - Any 64-bit process would use the v8 ABI all the time. When the binder device has been opened by a 64-bit process already, this forces all other processes opening it to also use v8. - An existing 32-bit process would keep using the v7 ABI, but as long as the the v7 interface is in use, no other process may use the v8 ABI. This would exclude all 64-bit processes, as well as prevent 32-bit processes other than the first one from switching binder to the v8 ABI - Future binder user space implementations on 32-bit include a call to a new ioctl for switching from v7 to v8. The binder user space calls this immediately after opening the device to tell the kernel that it wants to use the v8 ABI, and from that point on, it behaves like the first case (opened by a 64-bit process). To support both ABIs in the same kernel module, that has to convert between the data structures. This is not much different between the module parameter method and the ioctl switch. It can continue using the v8 structures internally and then do: static bool abi_v8 = !BINDER_IPC_32BIT; module_parm(abi_v8, bool, S_IRUGO); static long binder_ioctl_
Re: [PATCH] android: binder: fix type mismatch warning
On Fri, Sep 22, 2017 at 9:00 AM, Martijn Coenen wrote: >> The case that gets interesting is a any kind of user that wants to >> run an Android application on a regular Linux box without >> using virtual machines or emulation, e.g. a an app developer, >> or a user that wants to run some Android app on their desktop >> using anbox. > > I don't think the binder driver is present on any regular Linux box, > so things like anbox actually have to provide it as a module. It looks > like the current version of anbox is actually using the v8 interface, > and is also using a modified version of the binder driver (eg for > dkms, but also other hacks). The other popular one is Shashlik, but > looks like it uses a VM (just for binder). The v7 interface doesn't > work on 64-bit machines at all, so any container userspace using v7 > would be seriously limiting the number of machines it could be run on, > whereas v8 runs on both 32 and 64. So I'm not sure we have to be > concerned about things like this, certainly if we have communicated > earlier (as Greg said) that binder shouldn't be used outside Android > pre 4.14. Ok. >> The scenario I had in mind is like this: > > Thanks for clarifying! I think this works, but it's additional code to > maintain and support for a pretty rare (and unsupported?) scenario. I > understand that hiding ABI behind a config option is not a good > design, and that it must be removed. If we really can't remove v7 from > 4.14, I would actually prefer to leave the config option (but flip the > default to v8), and remove it as soon as we think it can be (eg once P > has been in the field for a while). How would waiting help? If we are reasonably sure that it's not a problem, then let's remove the v7 support now and see if anyone complains (and put it back if they did have a reason to need it). If we can assume that the upstream binder driver (unlike third-party modules for anbox, which are not affected as you say) is only ever used with a platform specific full Android build, the question is what upgrade scenarios a device might need to support. The relevant cases would seem to be: 1. updating a 32-bit Android Oreo (or earlier) OS from linux-4.9 (or earlier) to linux-4.14 (or later) without updating libbinder: This would require keeping around compile-time option at least, for as long new kernels are supported on Oreo. 2. updating a 32-bit kernel to a 64-bit kernel on hardware that allows this, without changing anything else: Theoretically fixable, but this would require significant code changes in the kernel module, similar to what I described above. 3. upgrading from Android Oreo (or earlier) to Android P while upgrading the kernel at the same time: should be no problem. 4. upgrading from Android Oreo (or earlier) to Android P libbinder without upgrading the kernel: this might require patching the old kernels to enable the v8 ABI. Not our problem here, as it only concerns older kernels. What assumptions can we make about the two problematic cases (1 and 2) here? Is it reasonable to require device makes to always update libbinder together with the kernel? Could that run into problems after a botched upgrade that reverts back to an older kernel but keeps the new user space or vice versa? Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
