[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 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
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] 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: 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
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
[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
[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] 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 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
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] 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
[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
[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: 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
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: 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 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
[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: [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
[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
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
[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 =>
[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
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 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
[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
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 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:
[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
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 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:
[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/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/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 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 =
Re: [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check
On Fri, May 29, 2020 at 10:23 PM Arnd Bergmann wrote: > > On Fri, May 29, 2020 at 10:04 PM 'Nick Desaulniers' via Clang Built > Linux wrote: > > > > See also Nathan's 7 patch series. > > https://lore.kernel.org/lkml/20200527071150.3381228-1-natechancel...@gmail.com/ > > > > Might be some overlap between series? > > > > Probably. I really should have checked when I saw the number of warnings. > > At least this gives Mauro a chance to double-check the changes and see if > Nathan and I came to different conclusions on any of them. I checked now and found that the overlap is smaller than I expected. In each case, Nathans' solution seems more complete than mine, so this patch ("staging: media: atomisp: fix incorrect NULL pointer check") and also "staging: media: atomisp: fix a type conversion warning" can be dropped, but I think the others are still needed. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check
On Fri, May 29, 2020 at 10:04 PM 'Nick Desaulniers' via Clang Built Linux wrote: > > See also Nathan's 7 patch series. > https://lore.kernel.org/lkml/20200527071150.3381228-1-natechancel...@gmail.com/ > > Might be some overlap between series? > Probably. I really should have checked when I saw the number of warnings. At least this gives Mauro a chance to double-check the changes and see if Nathan and I came to different conclusions on any of them. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check
Checking the pointer to a member of a struct against NULL is pointless as clang points out: drivers/staging/media/atomisp/pci/atomisp_cmd.c:4278:17: error: address of 'config->info' will always evaluate to 'true' Check the original pointer instead, which may also be unnecessary here, but makes a little more sense. Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy") Signed-off-by: Arnd Bergmann --- drivers/staging/media/atomisp/pci/atomisp_cmd.c | 2 +- drivers/staging/media/atomisp/pci/sh_css.c | 4 ++-- drivers/staging/media/atomisp/pci/sh_css_sp.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c index 5be690f876c1..342fc3b34fe0 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c @@ -4275,7 +4275,7 @@ int atomisp_param(struct atomisp_sub_device *asd, int flag, atomisp_css_get_dvs_grid_info( &asd->params.curr_grid_info); - if (!&config->info) { + if (!config) { dev_err(isp->dev, "ERROR: NULL pointer in grid_info\n"); return -EINVAL; } diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c index d77432254a2c..e91c6029c651 100644 --- a/drivers/staging/media/atomisp/pci/sh_css.c +++ b/drivers/staging/media/atomisp/pci/sh_css.c @@ -8534,7 +8534,7 @@ ia_css_pipe_load_extension(struct ia_css_pipe *pipe, if (firmware->info.isp.type == IA_CSS_ACC_OUTPUT) { - if (&pipe->output_stage) + if (pipe) append_firmware(&pipe->output_stage, firmware); else { IA_CSS_LEAVE_ERR_PRIVATE(IA_CSS_ERR_INTERNAL_ERROR); @@ -8542,7 +8542,7 @@ ia_css_pipe_load_extension(struct ia_css_pipe *pipe, } } else if (firmware->info.isp.type == IA_CSS_ACC_VIEWFINDER) { - if (&pipe->vf_stage) + if (pipe) append_firmware(&pipe->vf_stage, firmware); else { IA_CSS_LEAVE_ERR_PRIVATE(IA_CSS_ERR_INTERNAL_ERROR); diff --git a/drivers/staging/media/atomisp/pci/sh_css_sp.c b/drivers/staging/media/atomisp/pci/sh_css_sp.c index e574396ad0f4..c0e579c1705f 100644 --- a/drivers/staging/media/atomisp/pci/sh_css_sp.c +++ b/drivers/staging/media/atomisp/pci/sh_css_sp.c @@ -1022,7 +1022,7 @@ sh_css_sp_init_stage(struct ia_css_binary *binary, if (!pipe) return IA_CSS_ERR_INTERNAL_ERROR; ia_css_get_crop_offsets(pipe, &args->in_frame->info); - } else if (&binary->in_frame_info) + } else if (binary) { pipe = find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num); if (!pipe) @@ -1036,7 +1036,7 @@ sh_css_sp_init_stage(struct ia_css_binary *binary, if (!pipe) return IA_CSS_ERR_INTERNAL_ERROR; ia_css_get_crop_offsets(pipe, &args->in_frame->info); - } else if (&binary->in_frame_info) { + } else if (binary) { pipe = find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num); if (!pipe) return IA_CSS_ERR_INTERNAL_ERROR; -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 8/9] staging: media: atomisp: disable all custom formats
clang points out the usage of an incorrect enum type in the list of supported image formats: drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:65: error: implicit conversion from enumeration type 'enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion] { V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 }, drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:39: error: implicit conversion from enumeration type 'enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion] { V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 }, { V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0, CSS_FRAME_FORMAT_NV12 }, { MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 }, Checking the git history, I found a commit that disabled one such case because it did not work. It seems likely that the incorrect enum was part of the original problem and that the others do not work either, or have never been tested. Disable all the ones that cause a warning. Fixes: cb02ae3d71ea ("media: staging: atomisp: Disable custom format for now") Signed-off-by: Arnd Bergmann --- drivers/staging/media/atomisp/pci/atomisp_subdev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c index 46590129cbe3..8bce466cc128 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c @@ -44,9 +44,11 @@ const struct atomisp_in_fmt_conv atomisp_in_fmt_conv[] = { { MEDIA_BUS_FMT_SRGGB12_1X12, 12, 12, ATOMISP_INPUT_FORMAT_RAW_12, CSS_BAYER_ORDER_RGGB, CSS_FORMAT_RAW_12 }, { MEDIA_BUS_FMT_UYVY8_1X16, 8, 8, ATOMISP_INPUT_FORMAT_YUV422_8, 0, ATOMISP_INPUT_FORMAT_YUV422_8 }, { MEDIA_BUS_FMT_YUYV8_1X16, 8, 8, ATOMISP_INPUT_FORMAT_YUV422_8, 0, ATOMISP_INPUT_FORMAT_YUV422_8 }, +#if 0 { MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 }, { V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0, CSS_FRAME_FORMAT_NV12 }, { V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 }, +#endif { V4L2_MBUS_FMT_CUSTOM_YUV420, 12, 12, ATOMISP_INPUT_FORMAT_YUV420_8_LEGACY, 0, ATOMISP_INPUT_FORMAT_YUV420_8_LEGACY }, #if 0 { V4L2_MBUS_FMT_CUSTOM_M10MO_RAW, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 }, -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/9] staging: media: atomisp: fix enum type mixups
Some function calls pass an incorrect enum type: drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:858:16: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion] gp_device_rst(INPUT_SYSTEM0_ID); ~ ^~~~ drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:860:19: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion] input_switch_rst(INPUT_SYSTEM0_ID); ^~~~ drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:876:27: error: implicit conversion from enumeration type 'input_system_cfg_flag_t' to different enumeration type 'input_system_connection_t' [-Werror,-Wenum-conversion] config.multicast[i] = INPUT_SYSTEM_CFG_FLAG_RESET; ~ ^~~ drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1326:32: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion] input_selector_cfg_for_sensor(INPUT_SYSTEM0_ID); ~ ^~~~ drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1329:19: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion] input_switch_cfg(INPUT_SYSTEM0_ID, &config.input_switch_cfg); ^~~~ INPUT_SYSTEM0_ID is zero, so use the corresponding zero-value of the expected types instead. Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2") Signed-off-by: Arnd Bergmann --- .../pci/hive_isp_css_common/host/input_system.c| 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c index 2114cf4f3fda..aa0f0fca9346 100644 --- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c @@ -855,9 +855,9 @@ input_system_error_t input_system_configuration_reset(void) input_system_network_rst(INPUT_SYSTEM0_ID); - gp_device_rst(INPUT_SYSTEM0_ID); + gp_device_rst(GP_DEVICE0_ID); - input_switch_rst(INPUT_SYSTEM0_ID); + input_switch_rst(GP_DEVICE0_ID); //target_rst(); @@ -873,7 +873,7 @@ input_system_error_t input_system_configuration_reset(void) for (i = 0; i < N_CSI_PORTS; i++) { config.csi_buffer_flags[i] = INPUT_SYSTEM_CFG_FLAG_RESET; - config.multicast[i] = INPUT_SYSTEM_CFG_FLAG_RESET; + config.multicast[i] = INPUT_SYSTEM_DISCARD_ALL; } config.source_type_flags = INPUT_SYSTEM_CFG_FLAG_RESET; @@ -1323,10 +1323,10 @@ static input_system_error_t configuration_to_registers(void) } // end of switch (source_type) // Set input selector. - input_selector_cfg_for_sensor(INPUT_SYSTEM0_ID); + input_selector_cfg_for_sensor(GP_DEVICE0_ID); // Set input switch. - input_switch_cfg(INPUT_SYSTEM0_ID, &config.input_switch_cfg); + input_switch_cfg(GP_DEVICE0_ID, &config.input_switch_cfg); // Set input formatters. // AM: IF are set dynamically. -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/9] staging: media: atomisp: fix type mismatch
The caller passes a variable of a different enum type: drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c:1707:64: error: implicit conversion from enumeration type 'const enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion] binary_supports_input_format(xcandidate, req_in_info->format)); An earlier patch tried to address this by changing the type of the function argument, but as the caller passes two different arguments, there is still a warning. Assume that the last patch was correct and change the other caller as well. Fixes: 0116b8df1c9e ("media: staging: atomisp: stop duplicating input format types") Signed-off-by: Arnd Bergmann --- drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c index 2a23b7c6aeeb..10d698295daf 100644 --- a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c +++ b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c @@ -1704,7 +1704,7 @@ ia_css_binary_find(struct ia_css_binary_descr *descr, ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_binary_find() [%d] continue: !%d\n", __LINE__, - binary_supports_input_format(xcandidate, req_in_info->format)); + binary_supports_input_format(xcandidate, descr->stream_format)); continue; } #endif -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/9] staging: media: atomisp: fix a type conversion warning
clang complains that the type conversion in the MAX() macro contains an implied integer overflow to a signed number: drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c:129:35: error: implicit conversion from 'unsigned long' to 'int32_t' (aka 'int') changes value from 18446744073709543424 to -8192 [-Werror,-Wconstant-conversion] As the conversion is clearly intended here, use an explicit cast. Fixes: 9a0d7fb5ece6 ("media: atomisp: simplify math_support.h") Signed-off-by: Arnd Bergmann --- .../atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c index a9db6366d20b..613bace0522a 100644 --- a/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c +++ b/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c @@ -126,7 +126,7 @@ compute_blending(int strength) * exactly as s0.11 fixed point, but -1.0 can. */ isp_strength = -(((strength * isp_scale) + offset) / host_scale); - return MAX(MIN(isp_strength, 0), -XNR_BLENDING_SCALE_FACTOR); + return MAX(MIN(isp_strength, 0), -(unsigned int)XNR_BLENDING_SCALE_FACTOR); } void -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/9] staging: media: atomisp: fix stack overflow in init_pipe_defaults()
When building with clang, multiple copies of the structures to be initialized are passed around on the stack and copied locally, using an insane amount of stack space: drivers/staging/media/atomisp/pci/sh_css.c:2371:1: error: stack frame size of 26864 bytes in function 'create_pipe' [-Werror,-Wframe-larger-than=] Use constantly-allocated variables plus an explicit memcpy() to avoid that. Fixes: 6dc9a2568f84 ("media: atomisp: convert default struct values to use compound-literals with designated initializers") Signed-off-by: Arnd Bergmann --- drivers/staging/media/atomisp/pci/sh_css.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c index e91c6029c651..1e8b9d637116 100644 --- a/drivers/staging/media/atomisp/pci/sh_css.c +++ b/drivers/staging/media/atomisp/pci/sh_css.c @@ -2264,6 +2264,12 @@ static enum ia_css_err init_pipe_defaults(enum ia_css_pipe_mode mode, struct ia_css_pipe *pipe, bool copy_pipe) { + static const struct ia_css_pipe default_pipe = IA_CSS_DEFAULT_PIPE; + static const struct ia_css_preview_settings preview = IA_CSS_DEFAULT_PREVIEW_SETTINGS; + static const struct ia_css_capture_settings capture = IA_CSS_DEFAULT_CAPTURE_SETTINGS; + static const struct ia_css_video_settings video = IA_CSS_DEFAULT_VIDEO_SETTINGS; + static const struct ia_css_yuvpp_settings yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS; + if (!pipe) { IA_CSS_ERROR("NULL pipe parameter"); @@ -2271,14 +2277,14 @@ init_pipe_defaults(enum ia_css_pipe_mode mode, } /* Initialize pipe to pre-defined defaults */ - *pipe = IA_CSS_DEFAULT_PIPE; + memcpy(pipe, &default_pipe, sizeof(default_pipe)); /* TODO: JB should not be needed, but temporary backward reference */ switch (mode) { case IA_CSS_PIPE_MODE_PREVIEW: pipe->mode = IA_CSS_PIPE_ID_PREVIEW; - pipe->pipe_settings.preview = IA_CSS_DEFAULT_PREVIEW_SETTINGS; + memcpy(&pipe->pipe_settings.preview, &preview, sizeof(preview)); break; case IA_CSS_PIPE_MODE_CAPTURE: if (copy_pipe) { @@ -2286,11 +2292,11 @@ init_pipe_defaults(enum ia_css_pipe_mode mode, } else { pipe->mode = IA_CSS_PIPE_ID_CAPTURE; } - pipe->pipe_settings.capture = IA_CSS_DEFAULT_CAPTURE_SETTINGS; + memcpy(&pipe->pipe_settings.capture, &capture, sizeof(capture)); break; case IA_CSS_PIPE_MODE_VIDEO: pipe->mode = IA_CSS_PIPE_ID_VIDEO; - pipe->pipe_settings.video = IA_CSS_DEFAULT_VIDEO_SETTINGS; + memcpy(&pipe->pipe_settings.video, &video, sizeof(video)); break; case IA_CSS_PIPE_MODE_ACC: pipe->mode = IA_CSS_PIPE_ID_ACC; @@ -2300,7 +2306,7 @@ init_pipe_defaults(enum ia_css_pipe_mode mode, break; case IA_CSS_PIPE_MODE_YUVPP: pipe->mode = IA_CSS_PIPE_ID_YUVPP; - pipe->pipe_settings.yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS; + memcpy(&pipe->pipe_settings.yuvpp, &yuvpp, sizeof(yuvpp)); break; default: return IA_CSS_ERR_INVALID_ARGUMENTS; -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/9] staging: media: atomisp: annotate an unused function
atomisp_mrfld_power() has no more callers and produces a warning: drivers/staging/media/atomisp/pci/atomisp_v4l2.c:764:12: error: unused function 'atomisp_mrfld_power' [-Werror,-Wunused-function] Mark the function as unused while the PM code is being debugged, expecting that it will be used again in the future and should not just be removed. Fixes: 95d1f398c4dc ("media: atomisp: keep the ISP powered on when setting it") Signed-off-by: Arnd Bergmann --- drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 694268d133c0..10abb35ba0e0 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -761,7 +761,8 @@ static void punit_ddr_dvfs_enable(bool enable) pr_info("DDR DVFS, door bell is not cleared within 3ms\n"); } -static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable) +static __attribute__((unused)) int +atomisp_mrfld_power(struct atomisp_device *isp, bool enable) { unsigned long timeout; u32 val = enable ? MRFLD_ISPSSPM0_IUNIT_POWER_ON : -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 9/9] staging: media: atomisp: add PMIC_OPREGION dependency
Without that driver, there is a link failure in ERROR: modpost: "intel_soc_pmic_exec_mipi_pmic_seq_element" [drivers/staging/media/atomisp/pci/atomisp_gmin_platform.ko] undefined! Add an explicit Kconfig dependency. Signed-off-by: Arnd Bergmann --- drivers/staging/media/atomisp/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/media/atomisp/Kconfig b/drivers/staging/media/atomisp/Kconfig index c4f3049b0706..e86311c14329 100644 --- a/drivers/staging/media/atomisp/Kconfig +++ b/drivers/staging/media/atomisp/Kconfig @@ -11,6 +11,7 @@ menuconfig INTEL_ATOMISP config VIDEO_ATOMISP tristate "Intel Atom Image Signal Processor Driver" depends on VIDEO_V4L2 && INTEL_ATOMISP + depends on PMIC_OPREGION select IOSF_MBI select VIDEOBUF_VMALLOC ---help--- -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/9] staging: media: atomisp: declare 'struct device' before using it
In some configurations, including this header leads to a warning: drivers/staging/media/atomisp//pci/sh_css_firmware.h:41:38: error: declaration of 'struct device' will not be visible outside of this function [-Werror,-Wvisibility] Make sure the struct tag is known before declaring a function that uses it as an argument. Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy") Signed-off-by: Arnd Bergmann --- drivers/staging/media/atomisp/pci/sh_css_firmware.h | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.h b/drivers/staging/media/atomisp/pci/sh_css_firmware.h index f6253392a6c9..317559c7689f 100644 --- a/drivers/staging/media/atomisp/pci/sh_css_firmware.h +++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.h @@ -37,6 +37,7 @@ extern unsigned int sh_css_num_binaries; char *sh_css_get_fw_version(void); +struct device; bool sh_css_check_firmware_version(struct device *dev, const char *fw_data); -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: tegra-video: fix V4L2 dependency
Rather than using a dependency on VIDEO_V4L2, this driver uses "select", which fails when other dependencies are missing: WARNING: unmet direct dependencies detected for VIDEO_V4L2 Depends on [n]: MEDIA_SUPPORT [=y] && (I2C [=y] || I2C [=y]=n) && VIDEO_DEV [=n] Selected by [y]: - VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] && MEDIA_SUPPORT [=y] && TEGRA_HOST1X [=y] (plus an endless stream of link errors for other drivers that depend on VIDEO_V4L2 but are now lacking their dependencies) Fixes: 3d8a97eabef0 ("media: tegra-video: Add Tegra210 Video input driver") Signed-off-by: Arnd Bergmann --- drivers/staging/media/tegra-video/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/tegra-video/Kconfig b/drivers/staging/media/tegra-video/Kconfig index 3f03b5b39e6c..f6c61ec74386 100644 --- a/drivers/staging/media/tegra-video/Kconfig +++ b/drivers/staging/media/tegra-video/Kconfig @@ -2,7 +2,7 @@ config VIDEO_TEGRA tristate "NVIDIA Tegra VI driver" depends on TEGRA_HOST1X - select VIDEO_V4L2 + depends on VIDEO_V4L2 select MEDIA_CONTROLLER select VIDEOBUF2_DMA_CONTIG help -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wfx: avoid compiler warning on empty array
On Thu, Apr 30, 2020 at 10:42 AM Jerome Pouiller wrote: > On Wednesday 29 April 2020 22:34:56 CEST Arnd Bergmann wrote: > > On Wed, Apr 29, 2020 at 6:04 PM Jerome Pouiller > > wrote: > > > On Wednesday 29 April 2020 16:21:09 CEST Arnd Bergmann wrote: > > > > > > > > -static const struct of_device_id wfx_sdio_of_match[]; > > > > +static const struct of_device_id wfx_sdio_of_match[] = { > > > > + { .compatible = "silabs,wfx-sdio" }, > > > > + { .compatible = "silabs,wf200" }, > > > > + { }, > > > > +}; > > > > +MODULE_DEVICE_TABLE(of, wfx_sdio_of_match); > > > > > > I suggest to keep the '#ifdef CONFIG_OF' around this definition. If > > > CONFIG_OF is undefined, of_match_ptr() and of_match_node() will be NULL > > > and it should compile. > > > > I would generally always go for fewer #ifdef instead of more when the result > > is the same. Are you worried about wasting 600 bytes of object code size for > > the array on systems that need this driver but not CONFIG_OF, or something > > else? > > I am not very concerned about the size of the object. However, I think > that all the modules should apply the same policy regarding the device > tables. With a few greps, I found 3954 struct of_device_id. About 500 are > inside #ifdef and about 1000 use of_match_ptr(). > > Should we consider that the structs of_device_id have to be defined even > if CONFIG_OF is not defined? And In this case, should we drop > of_match_ptr()? > > Or in contrary, when kernel is compiled without CONFIG_OF, no modules > should contains OF entries in its device table? I think the drivers that use an #ifdef here just do so for historic reasons. In the linux-2.6 days, this caused build failures, but just leaving them defined has worked for a long time. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wfx: avoid compiler warning on empty array
On Wed, Apr 29, 2020 at 6:04 PM Jerome Pouiller wrote: > On Wednesday 29 April 2020 16:21:09 CEST Arnd Bergmann wrote: > > > > -static const struct of_device_id wfx_sdio_of_match[]; > > +static const struct of_device_id wfx_sdio_of_match[] = { > > + { .compatible = "silabs,wfx-sdio" }, > > + { .compatible = "silabs,wf200" }, > > + { }, > > +}; > > +MODULE_DEVICE_TABLE(of, wfx_sdio_of_match); > > I suggest to keep the '#ifdef CONFIG_OF' around this definition. If > CONFIG_OF is undefined, of_match_ptr() and of_match_node() will be NULL > and it should compile. I would generally always go for fewer #ifdef instead of more when the result is the same. Are you worried about wasting 600 bytes of object code size for the array on systems that need this driver but not CONFIG_OF, or something else? Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] greybus: uart: fix uninitialized flow control variable
gcc-10 points out an uninitialized variable use: drivers/staging/greybus/uart.c: In function 'gb_tty_set_termios': drivers/staging/greybus/uart.c:540:24: error: 'newline.flow_control' is used uninitialized in this function [-Werror=uninitialized] 540 | newline.flow_control |= GB_SERIAL_AUTO_RTSCTS_EN; Instead of using |= and &= on the uninitialized variable, use a direct assignment. Fixes: e55c25206d5c ("greybus: uart: Handle CRTSCTS flag in termios") Signed-off-by: Arnd Bergmann --- drivers/staging/greybus/uart.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index 55c51143bb09..4ffb334cd5cd 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -537,9 +537,9 @@ static void gb_tty_set_termios(struct tty_struct *tty, } if (C_CRTSCTS(tty) && C_BAUD(tty) != B0) - newline.flow_control |= GB_SERIAL_AUTO_RTSCTS_EN; + newline.flow_control = GB_SERIAL_AUTO_RTSCTS_EN; else - newline.flow_control &= ~GB_SERIAL_AUTO_RTSCTS_EN; + newline.flow_control = 0; if (memcmp(&gb_tty->line_coding, &newline, sizeof(newline))) { memcpy(&gb_tty->line_coding, &newline, sizeof(newline)); -- 2.26.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: wfx: avoid compiler warning on empty array
When CONFIG_OF is disabled, gcc-9 produces a warning about the wfx_sdio_of_match[] array having a declaration without a dimension: drivers/staging/wfx/bus_sdio.c:159:34: error: array 'wfx_sdio_of_match' assumed to have one element [-Werror] 159 | static const struct of_device_id wfx_sdio_of_match[]; | ^ Move the proper declaration up and out of the #ifdef instead. Fixes: a7a91ca5a23d ("staging: wfx: add infrastructure for new driver") Signed-off-by: Arnd Bergmann --- drivers/staging/wfx/bus_sdio.c | 19 --- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/drivers/staging/wfx/bus_sdio.c b/drivers/staging/wfx/bus_sdio.c index dedc3ff58d3e..c2e4bd1e3b0a 100644 --- a/drivers/staging/wfx/bus_sdio.c +++ b/drivers/staging/wfx/bus_sdio.c @@ -156,7 +156,13 @@ static const struct hwbus_ops wfx_sdio_hwbus_ops = { .align_size = wfx_sdio_align_size, }; -static const struct of_device_id wfx_sdio_of_match[]; +static const struct of_device_id wfx_sdio_of_match[] = { + { .compatible = "silabs,wfx-sdio" }, + { .compatible = "silabs,wf200" }, + { }, +}; +MODULE_DEVICE_TABLE(of, wfx_sdio_of_match); + static int wfx_sdio_probe(struct sdio_func *func, const struct sdio_device_id *id) { @@ -248,15 +254,6 @@ static const struct sdio_device_id wfx_sdio_ids[] = { }; MODULE_DEVICE_TABLE(sdio, wfx_sdio_ids); -#ifdef CONFIG_OF -static const struct of_device_id wfx_sdio_of_match[] = { - { .compatible = "silabs,wfx-sdio" }, - { .compatible = "silabs,wf200" }, - { }, -}; -MODULE_DEVICE_TABLE(of, wfx_sdio_of_match); -#endif - struct sdio_driver wfx_sdio_driver = { .name = "wfx-sdio", .id_table = wfx_sdio_ids, @@ -264,6 +261,6 @@ struct sdio_driver wfx_sdio_driver = { .remove = wfx_sdio_remove, .drv = { .owner = THIS_MODULE, - .of_match_table = of_match_ptr(wfx_sdio_of_match), + .of_match_table = wfx_sdio_of_match, } }; -- 2.26.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: staging: rkisp1: avoid unused variable warning
On Wed, Apr 8, 2020 at 7:56 PM Ezequiel Garcia wrote: > > On Wed, 2020-04-08 at 17:52 +0200, Arnd Bergmann wrote: > > When compile-testing with CONFIG_OF disabled, we get a warning > > about an unused variable, and about inconsistent Kconfig dependencies: > > > > WARNING: unmet direct dependencies detected for PHY_ROCKCHIP_DPHY_RX0 > > Depends on [n]: STAGING [=y] && STAGING_MEDIA [=y] && MEDIA_SUPPORT [=m] > > && (ARCH_ROCKCHIP [=n] || COMPILE_TEST [=y]) && OF [=n] > > Selected by [m]: > > - VIDEO_ROCKCHIP_ISP1 [=m] && STAGING [=y] && STAGING_MEDIA [=y] && > > MEDIA_SUPPORT [=m] && VIDEO_V4L2 [=m] && VIDEO_V4L2_SUBDEV_API [=y] && > > (ARCH_ROCKCHIP [=n] || COMPILE_TEST [=y]) > > > > drivers/staging/media/rkisp1/rkisp1-dev.c: In function 'rkisp1_probe': > > drivers/staging/media/rkisp1/rkisp1-dev.c:457:22: error: unused variable > > 'node' [-Werror=unused-variable] > > 457 | struct device_node *node = pdev->dev.of_node; > > > > Simply open-coding the pointer dereference in the only place > > the variable is used avoids the warning in all configurations, > > so we can allow compile-testing as well. > > > > Hello Arnd, > > Thanks for your patch. > > I believe this is already fixed here: > > https://patchwork.linuxtv.org/patch/62774/ > https://patchwork.linuxtv.org/patch/62775/ Ok, sorry for the duplicate. I only tested on mainline from a few days ago, so I must have missed it getting merged in the meantime. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: staging: rkisp1: avoid unused variable warning
When compile-testing with CONFIG_OF disabled, we get a warning about an unused variable, and about inconsistent Kconfig dependencies: WARNING: unmet direct dependencies detected for PHY_ROCKCHIP_DPHY_RX0 Depends on [n]: STAGING [=y] && STAGING_MEDIA [=y] && MEDIA_SUPPORT [=m] && (ARCH_ROCKCHIP [=n] || COMPILE_TEST [=y]) && OF [=n] Selected by [m]: - VIDEO_ROCKCHIP_ISP1 [=m] && STAGING [=y] && STAGING_MEDIA [=y] && MEDIA_SUPPORT [=m] && VIDEO_V4L2 [=m] && VIDEO_V4L2_SUBDEV_API [=y] && (ARCH_ROCKCHIP [=n] || COMPILE_TEST [=y]) drivers/staging/media/rkisp1/rkisp1-dev.c: In function 'rkisp1_probe': drivers/staging/media/rkisp1/rkisp1-dev.c:457:22: error: unused variable 'node' [-Werror=unused-variable] 457 | struct device_node *node = pdev->dev.of_node; Simply open-coding the pointer dereference in the only place the variable is used avoids the warning in all configurations, so we can allow compile-testing as well. Fixes: d65dd85281fb ("media: staging: rkisp1: add Rockchip ISP1 base driver") Signed-off-by: Arnd Bergmann --- drivers/staging/media/phy-rockchip-dphy-rx0/Kconfig | 2 +- drivers/staging/media/rkisp1/rkisp1-dev.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/media/phy-rockchip-dphy-rx0/Kconfig b/drivers/staging/media/phy-rockchip-dphy-rx0/Kconfig index bd0147624de1..dd5d4d741bdd 100644 --- a/drivers/staging/media/phy-rockchip-dphy-rx0/Kconfig +++ b/drivers/staging/media/phy-rockchip-dphy-rx0/Kconfig @@ -2,7 +2,7 @@ config PHY_ROCKCHIP_DPHY_RX0 tristate "Rockchip MIPI Synopsys DPHY RX0 driver" - depends on (ARCH_ROCKCHIP || COMPILE_TEST) && OF + depends on (ARCH_ROCKCHIP && OF) || COMPILE_TEST select GENERIC_PHY_MIPI_DPHY select GENERIC_PHY help diff --git a/drivers/staging/media/rkisp1/rkisp1-dev.c b/drivers/staging/media/rkisp1/rkisp1-dev.c index b1b3c058e957..5e7e797aad71 100644 --- a/drivers/staging/media/rkisp1/rkisp1-dev.c +++ b/drivers/staging/media/rkisp1/rkisp1-dev.c @@ -454,7 +454,6 @@ static void rkisp1_debug_init(struct rkisp1_device *rkisp1) static int rkisp1_probe(struct platform_device *pdev) { - struct device_node *node = pdev->dev.of_node; const struct rkisp1_match_data *clk_data; const struct of_device_id *match; struct device *dev = &pdev->dev; @@ -463,7 +462,7 @@ static int rkisp1_probe(struct platform_device *pdev) unsigned int i; int ret, irq; - match = of_match_node(rkisp1_of_match, node); + match = of_match_node(rkisp1_of_match, pdev->dev.of_node); rkisp1 = devm_kzalloc(dev, sizeof(*rkisp1), GFP_KERNEL); if (!rkisp1) return -ENOMEM; -- 2.26.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] vme: bridges: reduce stack usage
With CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE_O3, the stack usage in vme_fake grows above the warning limit: drivers/vme/bridges/vme_fake.c: In function 'fake_master_read': drivers/vme/bridges/vme_fake.c:610:1: error: the frame size of 1160 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] drivers/vme/bridges/vme_fake.c: In function 'fake_master_write': drivers/vme/bridges/vme_fake.c:797:1: error: the frame size of 1160 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] The problem is that in some configurations, each call to fake_vmereadX() puts another variable on the stack. Reduce the amount of inlining to get back to the previous state, with no function using more than 200 bytes each. Fixes: mmtom ("init/Kconfig: enable -O3 for all arches") Signed-off-by: Arnd Bergmann --- drivers/vme/bridges/vme_fake.c | 30 ++ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/drivers/vme/bridges/vme_fake.c b/drivers/vme/bridges/vme_fake.c index 3208a4409e44..6a1bc284f297 100644 --- a/drivers/vme/bridges/vme_fake.c +++ b/drivers/vme/bridges/vme_fake.c @@ -414,8 +414,9 @@ static void fake_lm_check(struct fake_driver *bridge, unsigned long long addr, } } -static u8 fake_vmeread8(struct fake_driver *bridge, unsigned long long addr, - u32 aspace, u32 cycle) +static noinline_for_stack u8 fake_vmeread8(struct fake_driver *bridge, + unsigned long long addr, + u32 aspace, u32 cycle) { u8 retval = 0xff; int i; @@ -446,8 +447,9 @@ static u8 fake_vmeread8(struct fake_driver *bridge, unsigned long long addr, return retval; } -static u16 fake_vmeread16(struct fake_driver *bridge, unsigned long long addr, - u32 aspace, u32 cycle) +static noinline_for_stack u16 fake_vmeread16(struct fake_driver *bridge, +unsigned long long addr, +u32 aspace, u32 cycle) { u16 retval = 0x; int i; @@ -478,8 +480,9 @@ static u16 fake_vmeread16(struct fake_driver *bridge, unsigned long long addr, return retval; } -static u32 fake_vmeread32(struct fake_driver *bridge, unsigned long long addr, - u32 aspace, u32 cycle) +static noinline_for_stack u32 fake_vmeread32(struct fake_driver *bridge, +unsigned long long addr, +u32 aspace, u32 cycle) { u32 retval = 0x; int i; @@ -609,8 +612,9 @@ static ssize_t fake_master_read(struct vme_master_resource *image, void *buf, return retval; } -static void fake_vmewrite8(struct fake_driver *bridge, u8 *buf, - unsigned long long addr, u32 aspace, u32 cycle) +static noinline_for_stack void fake_vmewrite8(struct fake_driver *bridge, + u8 *buf, unsigned long long addr, + u32 aspace, u32 cycle) { int i; unsigned long long start, end, offset; @@ -639,8 +643,9 @@ static void fake_vmewrite8(struct fake_driver *bridge, u8 *buf, } -static void fake_vmewrite16(struct fake_driver *bridge, u16 *buf, - unsigned long long addr, u32 aspace, u32 cycle) +static noinline_for_stack void fake_vmewrite16(struct fake_driver *bridge, + u16 *buf, unsigned long long addr, + u32 aspace, u32 cycle) { int i; unsigned long long start, end, offset; @@ -669,8 +674,9 @@ static void fake_vmewrite16(struct fake_driver *bridge, u16 *buf, } -static void fake_vmewrite32(struct fake_driver *bridge, u32 *buf, - unsigned long long addr, u32 aspace, u32 cycle) +static noinline_for_stack void fake_vmewrite32(struct fake_driver *bridge, + u32 *buf, unsigned long long addr, + u32 aspace, u32 cycle) { int i; unsigned long long start, end, offset; -- 2.20.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188: avoid excessive stack usage
The rtl8188 copy of the os_dep support code causes a warning about a very significant stack usage in the translate_scan() function: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c: In function 'translate_scan': drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:306:1: error: the frame size of 1560 bytes is larger than 1400 bytes [-Werror=frame-larger-than=] Use the same trick as in the rtl8723bs copy of the same function, and allocate it dynamically. Signed-off-by: Arnd Bergmann --- drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c index 710c33fd4965..47f4cc6a19a9 100644 --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c @@ -222,18 +222,21 @@ static char *translate_scan(struct adapter *padapter, /* parsing WPA/WPA2 IE */ { - u8 buf[MAX_WPA_IE_LEN]; + u8 *buf; u8 wpa_ie[255], rsn_ie[255]; u16 wpa_len = 0, rsn_len = 0; u8 *p; + buf = kzalloc(MAX_WPA_IE_LEN, GFP_ATOMIC); + if (!buf) + return start; + rtw_get_sec_ie(pnetwork->network.ies, pnetwork->network.ie_length, rsn_ie, &rsn_len, wpa_ie, &wpa_len); RT_TRACE(_module_rtl871x_mlme_c_, _drv_info_, ("rtw_wx_get_scan: ssid =%s\n", pnetwork->network.ssid.ssid)); RT_TRACE(_module_rtl871x_mlme_c_, _drv_info_, ("rtw_wx_get_scan: wpa_len =%d rsn_len =%d\n", wpa_len, rsn_len)); if (wpa_len > 0) { p = buf; - memset(buf, 0, MAX_WPA_IE_LEN); p += sprintf(p, "wpa_ie="); for (i = 0; i < wpa_len; i++) p += sprintf(p, "%02x", wpa_ie[i]); @@ -250,7 +253,6 @@ static char *translate_scan(struct adapter *padapter, } if (rsn_len > 0) { p = buf; - memset(buf, 0, MAX_WPA_IE_LEN); p += sprintf(p, "rsn_ie="); for (i = 0; i < rsn_len; i++) p += sprintf(p, "%02x", rsn_ie[i]); @@ -264,6 +266,7 @@ static char *translate_scan(struct adapter *padapter, iwe.u.data.length = rsn_len; start = iwe_stream_add_point(info, start, stop, &iwe, rsn_ie); } + kfree(buf); } {/* parsing WPS IE */ -- 2.20.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 00/24] drivers, fs: 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. I will add these to my y2038 branch [1] for linux-next, but can keep rebasing for feedback and to remove any patches that get picked up by a maintainer. Changes since v1 [2]: - Add Acks I received - Rebase to v5.5-rc1, droping patches that got merged already - Add NFS, XFS and the final three patches from another series - Rewrite etnaviv patches Arnd [1] https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=y2038 [2] https://lore.kernel.org/lkml/20191108213257.3097633-1-a...@arndb.de/ Arnd Bergmann (24): Input: input_event: fix struct padding on sparc64 fat: use prandom_u32() for i_generation 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 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: reject timeouts with tv_nsec >= NSEC_PER_SEC drm/etnaviv: avoid deprecated timespec sunrpc: convert to time64_t for expiry nfs: use time64_t internally nfs: fix timstamp debug prints nfs: fscache: use timespec64 in inode auxdata xfs: rename compat_time_t to old_time32_t xfs: disallow broken ioctls without compat-32-bit-time xfs: quota: move to time64_t interfaces y2038: remove obsolete jiffies conversion functions y2038: rename itimerval to __kernel_old_itimerval y2038: sparc: remove use of struct timex arch/sparc/kernel/sys_sparc_64.c | 29 +- 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/gpu/drm/etnaviv/etnaviv_drv.c | 20 --- drivers/gpu/drm/etnaviv/etnaviv_drv.h | 11 ++-- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 4 +- drivers/gpu/drm/etnaviv/etnaviv_gem.h | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 5 +- drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 5 +- drivers/gpu/drm/msm/msm_drv.h | 3 +- drivers/input/evdev.c | 14 ++--- drivers/input/misc/uinput.c | 14 +++-- fs/dlm/lowcomms.c | 6 +- fs/fat/inode.c| 3 +- fs/hfs/hfs_fs.h | 28 +++-- fs/hfs/inode.c| 4 +- fs/hfsplus/hfsplus_fs.h | 28 +++-- fs/hfsplus/inode.c| 12 ++-- fs/hostfs/hostfs.h| 22 --- fs/hostfs/hostfs_kern.c | 15 +++-- fs/nfs/fscache-index.c| 6 +- fs/nfs/fscache.c | 18 -- fs/nfs/fscache.h | 8 ++- fs/nfs/nfs4xdr.c | 10 ++-- fs/quota/quotaio_v1.h | 6 +- fs/xfs/xfs_dquot.c| 6 +- fs/xfs/xfs_ioctl.c| 26 + fs/xfs/xfs_ioctl32.c | 2 +- fs/xfs/xfs_ioctl32.h | 2 +- fs/xfs/xfs_qm.h | 6 +- fs/xfs/xfs_quotaops.c | 6 +- fs/xfs/xfs_trans_dquot.c | 8 ++- include/linux/jiffies.h | 20 --- include/linux/sunrpc/cache.h | 42 -- include/linux/sunrpc/gss_api.h| 4 +- include/linux/sunrpc/gss_krb5.h | 2 +- include/linux/syscalls.h | 9 ++- include/uapi/linux/acct.h | 2 + include/uapi/linux/input.h| 1 + include/uapi/linux/taskstats.h| 6 +- include/uapi/linux/time_types.h | 5 ++ include/uapi/linux/timex.h| 2 + kernel/acct.c | 4 +- kernel/time/itimer.c | 18 +++--- kernel/time/time.c| 58 ++- kernel/tsacct.c | 9 ++- net/packet/af_packet.c| 27 + net/sunrpc/auth_gss/gss_krb5_mech.c | 12 +++- n
Re: [PATCH 1/2] staging: remove isdn capi drivers
On Tue, Dec 10, 2019 at 10:19 AM Greg Kroah-Hartman wrote: > > On Mon, Dec 09, 2019 at 04:11:13PM +0100, Arnd Bergmann wrote: > > As described in drivers/staging/isdn/TODO, the drivers are all > > assumed to be unmaintained and unused now, with gigaset being the > > last one to stop being maintained after Paul Bolle lost access > > to an ISDN network. > > > > The CAPI subsystem remains for now, as it is still required by > > bluetooth/cmtp. > > > > Signed-off-by: Arnd Bergmann > > --- > > Documentation/ioctl/ioctl-number.rst|1 - > > This file is not in 5.5-rc1, what tree did you make this against? This was against v5.4, sending a rebased version now. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: hp100: add back CONFIG_NET dependency
The move to staging lost an important dependency: ERROR: "eth_validate_addr" [drivers/staging/hp/hp100.ko] undefined! ERROR: "eth_mac_addr" [drivers/staging/hp/hp100.ko] undefined! ERROR: "alloc_etherdev_mqs" [drivers/staging/hp/hp100.ko] undefined! ERROR: "register_netdev" [drivers/staging/hp/hp100.ko] undefined! ERROR: "__skb_pad" [drivers/staging/hp/hp100.ko] undefined! ERROR: "consume_skb" [drivers/staging/hp/hp100.ko] undefined! ERROR: "dev_trans_start" [drivers/staging/hp/hp100.ko] undefined! ERROR: "__dev_kfree_skb_any" [drivers/staging/hp/hp100.ko] undefined! ERROR: "netif_rx" [drivers/staging/hp/hp100.ko] undefined! ERROR: "eth_type_trans" [drivers/staging/hp/hp100.ko] undefined! ERROR: "skb_trim" [drivers/staging/hp/hp100.ko] undefined! ERROR: "skb_put" [drivers/staging/hp/hp100.ko] undefined! ERROR: "__netdev_alloc_skb" [drivers/staging/hp/hp100.ko] undefined! ERROR: "free_netdev" [drivers/staging/hp/hp100.ko] undefined! ERROR: "unregister_netdev" [drivers/staging/hp/hp100.ko] undefined! Add it back explicitly. Fixes: 52340b82cf1a ("hp100: Move 100BaseVG AnyLAN driver to staging") Signed-off-by: Arnd Bergmann --- drivers/staging/hp/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/hp/Kconfig b/drivers/staging/hp/Kconfig index fb395cfe6b92..9566d4ab5ce5 100644 --- a/drivers/staging/hp/Kconfig +++ b/drivers/staging/hp/Kconfig @@ -6,6 +6,7 @@ config NET_VENDOR_HP bool "HP devices" default y + depends on NET depends on ISA || EISA || PCI ---help--- If you have a network (Ethernet) card belonging to this class, say Y. -- 2.20.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] [RFC] staging/net: move AF_X25 into drivers/staging
On Tue, Dec 10, 2019 at 9:59 AM Martin Schiller wrote: > On 2019-12-09 20:26, Arnd Bergmann wrote: > > On Mon, Dec 9, 2019 at 7:29 PM David Miller > > wrote: > >> > >> From: Arnd Bergmann > >> Date: Mon, 9 Dec 2019 16:12:56 +0100 > >> > >> > syzbot keeps finding issues in the X.25 implementation that nobody is > >> > interested in fixing. Given that all the x25 patches of the past years > >> > that are not global cleanups tend to fix user-triggered oopses, is it > >> > time to just retire the subsystem? > >> > >> I have a bug fix that I'm currently applying to 'net' right now > >> actually: > >> > >> https://patchwork.ozlabs.org/patch/1205973/ > >> > >> So your proposal might be a bit premature. > > > > Ok, makes sense. Looking back in the history, I also see other bugfixes > > from the same author. > > > > Adding Martin Schiller to Cc: for a few questions: > > > > - What hardware are you using for X.25? > > I would say that X.25 is (at least in Germany) not dead yet. For > example, it is still used in the railway network of the Deutsche Bahn AG > in many different areas. [1] > > We deliver products for this and use the Linux X.25 stack with some > bugfixes and extensions that I would like to get upstream. Right, when I looked for possible users, I found several examples where X.25 is still relevant, my impression was just that none of those were using the mainline Linux network stack. Thank you for clarifying that. > As hardware/interfaces we use X.21bis/G.703 adapters, which are > connected via > HDLC_X25 and LAPB. Also for this there are extensions and bugfixes, > which I would like to include in the kernel. > > - Would you be available to be listed in the MAINTAINERS file > > as a contact for net/x25? > > Yes, you can add me to the MAINTAINERS file. > I have only limited time, but I will try to follow all requests > concerning this subsystem. Great! I don't expect there to be a lot of work, but it definitely helps to have someone who can look at the occasional build failure or code cleanup patch. If this works for everyone, I'd submit the following patch: commit b63caa9a8d86a5bfc64052bf9aab9b22181120fd (HEAD) Author: Arnd Bergmann Date: Tue Dec 10 14:28:39 2019 +0100 MAINTAINERS: add new X.25 maintainer Martin Schiller is using the Linux X.25 stack on top of HDLC and X.21 networks. He agreed to be listed as a maintainer to take care of odd fixes. Add him as the primary contact for net/x25 and net/lapb, as well as a reviewer for drivers/net/wan, which contains the HDLC code. Cc: Martin Schiller Cc: Andrew Hendry Cc: Krzysztof Halasa Link: https://lore.kernel.org/netdev/407acd92c92c3ba04578da89b1a0f...@dev.tdt.de/ Signed-off-by: Arnd Bergmann diff --git a/MAINTAINERS b/MAINTAINERS index 8e58410a799a..00b624b96103 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6889,6 +6889,7 @@ F:Documentation/i2c/muxes/i2c-mux-gpio.rst GENERIC HDLC (WAN) DRIVERS M: Krzysztof Halasa +R: Martin Schiller W: http://www.kernel.org/pub/linux/utils/net/hdlc/ S: Maintained F: drivers/net/wan/c101.c @@ -9255,13 +9256,6 @@ S: Maintained F: arch/mips/lantiq F: drivers/soc/lantiq -LAPB module -L: linux-...@vger.kernel.org -S: Orphan -F: Documentation/networking/lapb-module.txt -F: include/*/lapb.h -F: net/lapb/ - LASI 53c700 driver for PARISC M: "James E.J. Bottomley" L: linux-s...@vger.kernel.org @@ -17911,11 +17905,16 @@ S:Maintained N: axp[128] X.25 NETWORK LAYER +M: Martin Schiller M: Andrew Hendry L: linux-...@vger.kernel.org S: Odd Fixes F: Documentation/networking/x25* +F: Documentation/networking/lapb-module.txt +F: include/linux/lapb.h F: include/net/x25* +F: include/uapi/linux/x25.h +F: net/lapb/ F: net/x25/ X86 ARCHITECTURE (32-BIT AND 64-BIT) - > > - Does your bug fix address the latest issue found by syzbot[1], > > or do you have an idea to fix it if not? > > I don't have a direct solution for the concrete problem mentioned above, > but at > first sight I would say that the commit 95d6ebd53c79 ("net/x25: fix > use-after-free in x25_device_event()") holds the wrong lock > (&x25_list_lock). > Shouldn't this be the lock &x25_neigh_list_lock as in x25_get_neigh(), > where x25_neigh_hold() is called? After looking at it again, my best guess is something else: x25_wait_for_connection_establishment() calls release_sock()/lock_sock() while waiting. At this point, a concurrent x25_connect() can overwrite the x25->neighbour variable, which needs to be checked again before calling x25_neigh_put(). Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] [RFC] staging/net: move AF_X25 into drivers/staging
On Mon, Dec 9, 2019 at 7:29 PM David Miller wrote: > > From: Arnd Bergmann > Date: Mon, 9 Dec 2019 16:12:56 +0100 > > > syzbot keeps finding issues in the X.25 implementation that nobody is > > interested in fixing. Given that all the x25 patches of the past years > > that are not global cleanups tend to fix user-triggered oopses, is it > > time to just retire the subsystem? > > I have a bug fix that I'm currently applying to 'net' right now actually: > > https://patchwork.ozlabs.org/patch/1205973/ > > So your proposal might be a bit premature. Ok, makes sense. Looking back in the history, I also see other bugfixes from the same author. Adding Martin Schiller to Cc: for a few questions: - What hardware are you using for X.25? - Would you be available to be listed in the MAINTAINERS file as a contact for net/x25? - Does your bug fix address the latest issue found by syzbot[1], or do you have an idea to fix it if not? Arnd [1] https://lore.kernel.org/netdev/cak8p3a0ldf+aq1hnzrvkknbqaum0wqw1jyr7_eb+jriwyhw...@mail.gmail.com/ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] [net-next] wan: remove stale Kconfig entries
The dscc4 driver was recently removed, but these Kconfig entries remain, so remove them as well. Fixes: 28c9eb9042a9 ("net/wan: dscc4: remove broken dscc4 driver") Signed-off-by: Arnd Bergmann --- drivers/net/wan/Kconfig | 24 1 file changed, 24 deletions(-) diff --git a/drivers/net/wan/Kconfig b/drivers/net/wan/Kconfig index bf2fe1d602ea..59b25d7e13e8 100644 --- a/drivers/net/wan/Kconfig +++ b/drivers/net/wan/Kconfig @@ -289,30 +289,6 @@ config SLIC_DS26522 To compile this driver as a module, choose M here: the module will be called slic_ds26522. -config DSCC4_PCISYNC - bool "Etinc PCISYNC features" - depends on DSCC4 - help - Due to Etinc's design choice for its PCISYNC cards, some operations - are only allowed on specific ports of the DSCC4. This option is the - only way for the driver to know that it shouldn't return a success - code for these operations. - - Please say Y if your card is an Etinc's PCISYNC. - -config DSCC4_PCI_RST - bool "Hard reset support" - depends on DSCC4 - help - Various DSCC4 bugs forbid any reliable software reset of the ASIC. - As a replacement, some vendors provide a way to assert the PCI #RST - pin of DSCC4 through the GPIO port of the card. If you choose Y, - the driver will make use of this feature before module removal - (i.e. rmmod). The feature is known to be available on Commtech's - cards. Contact your manufacturer for details. - - Say Y if your card supports this feature. - config IXP4XX_HSS tristate "Intel IXP4xx HSS (synchronous serial port) support" depends on HDLC && IXP4XX_NPE && IXP4XX_QMGR -- 2.20.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] [RFC] staging/net: move AF_X25 into drivers/staging
syzbot keeps finding issues in the X.25 implementation that nobody is interested in fixing. Given that all the x25 patches of the past years that are not global cleanups tend to fix user-triggered oopses, is it time to just retire the subsystem? I looked a bit closer and found: - we used to support x25 hardware in linux, but with WAN_ROUTER removed in linux-3.9 and isdn4linux removed in 5.3, there is only hdlc, ethernet and the N_X25 tty ldisc left. Out of these, only HDLC_X25 made it beyond the experimental stage, so this is probably what everyone uses if there are users at all. - The most common hdlc hardware that people seem to be using are the "farsync" PCIe and USB adapters. Linux only has drivers for the older PCI devices from that series, but no hardware that works on modern systems. - The manufacturer still updates their own kernel drivers and provides support, but ships that with a fork or rewrite of the subsystem code now. Kevin Curtis is also listed as maintainer, but appears to have given up in 2013 after [1]. - The most popular software implementation appears to be X25 over TCP (XOT), which is supported by Farsite and other out-of-tree stacks but never had an implementation in mainline. - Most other supported HDLC hardware that we supoprt is for the ISA or PCI buses. There are newer PCIe or USB devices, but those all require a custom device driver and often a custom subsystem, none of which got submitted for mainline inclusion. This includes hardware from Microgate (SyncLink), Comtrol (RocketPort Express) and Sealevel (SeaMAC). - The X.25 subsystem is listed as "odd fixes", but the last reply on the netdev mailing list from the maintainer was also in 2013[2]. - The HDLC subsystem itself is listed as maintained by Krzysztof Halasa, and there were new drivers merged for SoC based devices as late as 2016 by Zhao Qiang: Freescale/NXP QUICC Engine and Maxim ds26522. There has not been much work on HDLC or drivers/net/wan recently, but both developers are still responsive on the mailing list and work on other parts of the kernel. Based on the above, I would conclude that X.25 can probably get moved to staging as keeping it in the kernel seems to do more harm than good, but HDLC below it should probably stay as there it seems there are still users of a small subset of the mainline drivers. Move all of X.25 into drivers/staging for now, with a projected removal date set for Linux-5.8. Cc: Eric Biggers Cc: Andrew Hendry Cc: linux-...@vger.kernel.org Cc: Kevin Curtis Cc: "R.J.Dunlop" Cc: Zhao Qiang Cc: Krzysztof Halasa Reported-by: syzbot+429c200ffc8772bfe...@syzkaller.appspotmail.com Reported-by: syzbot+eec0c87f31a7c3b66...@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?id=5b0ecf0386f56be7fe7210a14d0f62df765c0c39 Signed-off-by: Arnd Bergmann If anyone has different views or additional information, let us know. If you agree with the above, please Ack. --- MAINTAINERS | 8 +- drivers/net/wan/Kconfig | 43 drivers/net/wan/Makefile | 3 - drivers/staging/Kconfig | 2 + drivers/staging/Makefile | 2 + .../x25/Documentation}/lapb-module.txt| 0 .../staging/x25/Documentation}/x25-iface.txt | 0 .../staging/x25/Documentation}/x25.txt| 0 drivers/staging/x25/Kconfig | 97 +++ {net => drivers/staging}/x25/Makefile | 8 ++ drivers/staging/x25/TODO | 15 +++ {net => drivers/staging}/x25/af_x25.c | 2 +- drivers/{net/wan => staging/x25}/hdlc_x25.c | 2 +- {include/net => drivers/staging/x25}/lapb.h | 2 +- .../lapb => drivers/staging/x25}/lapb_iface.c | 2 +- {net/lapb => drivers/staging/x25}/lapb_in.c | 2 +- {net/lapb => drivers/staging/x25}/lapb_out.c | 2 +- {net/lapb => drivers/staging/x25}/lapb_subr.c | 2 +- .../lapb => drivers/staging/x25}/lapb_timer.c | 2 +- drivers/{net/wan => staging/x25}/lapbether.c | 2 +- .../staging/x25/linux-lapb.h | 0 {net => drivers/staging}/x25/sysctl_net_x25.c | 2 +- .../x25.h => drivers/staging/x25/uapi-x25.h | 0 {include/net => drivers/staging/x25}/x25.h| 2 +- drivers/{net/wan => staging/x25}/x25_asy.c| 2 +- drivers/{net/wan => staging/x25}/x25_asy.h| 0 {net => drivers/staging}/x25/x25_dev.c| 2 +- {net => drivers/staging}/x25/x25_facilities.c | 2 +- {net => drivers/staging}/x25/x25_forward.c| 2 +- {net => drivers/staging}/x25/x25_in.c | 2 +- {net => drivers/staging}/x25/x25_link.c | 2 +- {net => drivers/staging}/x25/x25_out.c| 2 +- {net => drivers/staging}/x25/x25_proc.c | 2 +- {net => drivers/staging}/x25/x25_route.c | 2 +- {net => drivers
[PATCH 3/4] [net-next] wan: remove old frame relay driver
The DLCI framework and SDLA driver are as obsolete as it gets, this is an ISA add-on card from the mid-1990s that has only seen sporadic cleanups. The website listed for downloading the user space tools has dropped off the internet at some point around 2010, but Debian already dropped the packages ten years earlier when they no longer built correctly. Signed-off-by: Arnd Bergmann --- Documentation/networking/framerelay.txt | 39 - MAINTAINERS |6 - drivers/net/wan/Kconfig | 45 - drivers/net/wan/Makefile|2 - drivers/net/wan/dlci.c | 542 drivers/net/wan/sdla.c | 1655 --- include/Kbuild |1 - include/linux/if_frad.h | 92 -- include/linux/sdla.h| 240 include/uapi/linux/if_frad.h| 123 -- include/uapi/linux/sdla.h | 117 -- net/socket.c| 25 - 12 files changed, 2887 deletions(-) delete mode 100644 Documentation/networking/framerelay.txt delete mode 100644 drivers/net/wan/dlci.c delete mode 100644 drivers/net/wan/sdla.c delete mode 100644 include/linux/if_frad.h delete mode 100644 include/linux/sdla.h delete mode 100644 include/uapi/linux/if_frad.h delete mode 100644 include/uapi/linux/sdla.h diff --git a/Documentation/networking/framerelay.txt b/Documentation/networking/framerelay.txt deleted file mode 100644 index 1a0b720440dd.. --- a/Documentation/networking/framerelay.txt +++ /dev/null @@ -1,39 +0,0 @@ -Frame Relay (FR) support for linux is built into a two tiered system of device -drivers. The upper layer implements RFC1490 FR specification, and uses the -Data Link Connection Identifier (DLCI) as its hardware address. Usually these -are assigned by your network supplier, they give you the number/numbers of -the Virtual Connections (VC) assigned to you. - -Each DLCI is a point-to-point link between your machine and a remote one. -As such, a separate device is needed to accommodate the routing. Within the -net-tools archives is 'dlcicfg'. This program will communicate with the -base "DLCI" device, and create new net devices named 'dlci00', 'dlci01'... -The configuration script will ask you how many DLCIs you need, as well as -how many DLCIs you want to assign to each Frame Relay Access Device (FRAD). - -The DLCI uses a number of function calls to communicate with the FRAD, all -of which are stored in the FRAD's private data area. assoc/deassoc, -activate/deactivate and dlci_config. The DLCI supplies a receive function -to the FRAD to accept incoming packets. - -With this initial offering, only 1 FRAD driver is available. With many thanks -to Sangoma Technologies, David Mandelstam & Gene Kozin, the S502A, S502E & -S508 are supported. This driver is currently set up for only FR, but as -Sangoma makes more firmware modules available, it can be updated to provide -them as well. - -Configuration of the FRAD makes use of another net-tools program, 'fradcfg'. -This program makes use of a configuration file (which dlcicfg can also read) -to specify the types of boards to be configured as FRADs, as well as perform -any board specific configuration. The Sangoma module of fradcfg loads the -FR firmware into the card, sets the irq/port/memory information, and provides -an initial configuration. - -Additional FRAD device drivers can be added as hardware is available. - -At this time, the dlcicfg and fradcfg programs have not been incorporated into -the net-tools distribution. They can be found at ftp.invlogic.com, in -/pub/linux. Note that with OS/2 FTPD, you end up in /pub by default, so just -use 'cd linux'. v0.10 is for use on pre-2.0.3 and earlier, v0.15 is for -pre-2.0.4 and later. - diff --git a/MAINTAINERS b/MAINTAINERS index 0a996ac26047..32fab6fbd301 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6436,12 +6436,6 @@ W: http://floatingpoint.sourceforge.net/emulator/index.html S: Maintained F: arch/x86/math-emu/ -FRAME RELAY DLCI/FRAD (Sangoma drivers too) -L: net...@vger.kernel.org -S: Orphan -F: drivers/net/wan/dlci.c -F: drivers/net/wan/sdla.c - FRAMEBUFFER LAYER M: Bartlomiej Zolnierkiewicz L: dri-de...@lists.freedesktop.org diff --git a/drivers/net/wan/Kconfig b/drivers/net/wan/Kconfig index e9ad63115144..74f2bd639b07 100644 --- a/drivers/net/wan/Kconfig +++ b/drivers/net/wan/Kconfig @@ -297,51 +297,6 @@ config IXP4XX_HSS Say Y here if you want to use built-in HSS ports on IXP4xx processor. -config DLCI - tristate "Frame Relay DLCI support" - ---help--- - Support for the Frame Relay protocol. - - Frame Relay is a fast low-cost way to connect to a remote Internet - access provider or to form a private wide area net
[PATCH 2/4] [net-next] wan: remove sbni/granch driver
The driver was merged in 1999 and has only ever seen treewide cleanups since then, with no indication whatsoever that anyone has actually had access to hardware for testing the patches. >From the information in the link below, it appears that the hardware is for some leased line system in Russia that has since been discontinued. As the driver still feels like a Linux-2.2 era artifact today, it appears that the best way forward is to just delete it. Link: https://www.tms.ru/%D0%90%D0%B4%D0%B0%D0%BF%D1%82%D0%B5%D1%80_%D0%B4%D0%BB%D1%8F_%D0%B2%D1%8B%D0%B4%D0%B5%D0%BB%D0%B5%D0%BD%D0%BD%D1%8B%D1%85_%D0%BB%D0%B8%D0%BD%D0%B8%D0%B9_Granch_SBNI12-10 Signed-off-by: Arnd Bergmann --- .../admin-guide/kernel-parameters.txt |2 - drivers/net/Space.c |4 - drivers/net/wan/Kconfig | 28 - drivers/net/wan/Makefile |1 - drivers/net/wan/sbni.c| 1623 - drivers/net/wan/sbni.h| 147 -- include/net/Space.h |3 - 7 files changed, 1808 deletions(-) delete mode 100644 drivers/net/wan/sbni.c delete mode 100644 drivers/net/wan/sbni.h diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index a84a83f8881e..2161e4f93e17 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -4264,8 +4264,6 @@ sa1100ir[NET] See drivers/net/irda/sa1100_ir.c. - sbni= [NET] Granch SBNI12 leased line adapter - sched_debug [KNL] Enables verbose scheduler debug messages. schedstats= [KNL,X86] Enable or disable scheduled statistics. diff --git a/drivers/net/Space.c b/drivers/net/Space.c index 890c86e11bcc..7bb699d7c422 100644 --- a/drivers/net/Space.c +++ b/drivers/net/Space.c @@ -133,10 +133,6 @@ static int __init net_olddevs_init(void) { int num; -#ifdef CONFIG_SBNI - for (num = 0; num < 8; ++num) - sbni_probe(num); -#endif for (num = 0; num < 8; ++num) ethif_probe2(num); diff --git a/drivers/net/wan/Kconfig b/drivers/net/wan/Kconfig index 59b25d7e13e8..e9ad63115144 100644 --- a/drivers/net/wan/Kconfig +++ b/drivers/net/wan/Kconfig @@ -374,32 +374,4 @@ config X25_ASY If unsure, say N. -config SBNI - tristate "Granch SBNI12 Leased Line adapter support" - depends on X86 - ---help--- - Driver for ISA SBNI12-xx cards which are low cost alternatives to - leased line modems. - - You can find more information and last versions of drivers and - utilities at <http://www.granch.ru/>. If you have any question you - can send email to . - - To compile this driver as a module, choose M here: the - module will be called sbni. - - If unsure, say N. - -config SBNI_MULTILINE - bool "Multiple line feature support" - depends on SBNI - help - Schedule traffic for some parallel lines, via SBNI12 adapters. - - If you have two computers connected with two parallel lines it's - possible to increase transfer rate nearly twice. You should have - a program named 'sbniconfig' to configure adapters. - - If unsure, say N. - endif # WAN diff --git a/drivers/net/wan/Makefile b/drivers/net/wan/Makefile index 701f5d2fe3b6..3059b9a279b3 100644 --- a/drivers/net/wan/Makefile +++ b/drivers/net/wan/Makefile @@ -25,7 +25,6 @@ obj-$(CONFIG_LANMEDIA)+= lmc/ obj-$(CONFIG_DLCI) += dlci.o obj-$(CONFIG_SDLA) += sdla.o obj-$(CONFIG_LAPBETHER)+= lapbether.o -obj-$(CONFIG_SBNI) += sbni.o obj-$(CONFIG_N2) += n2.o obj-$(CONFIG_C101) += c101.o obj-$(CONFIG_WANXL)+= wanxl.o diff --git a/drivers/net/wan/sbni.c b/drivers/net/wan/sbni.c deleted file mode 100644 index 40c04ea1200a.. --- a/drivers/net/wan/sbni.c +++ /dev/null @@ -1,1623 +0,0 @@ -/* sbni.c: Granch SBNI12 leased line adapters driver for linux - * - * Written 2001 by Denis I.Timofeev (timof...@granch.ru) - * - * Previous versions were written by Yaroslav Polyakov, - * Alexey Zverev and Max Khon. - * - * Driver supports SBNI12-02,-04,-05,-10,-11 cards, single and - * double-channel, PCI and ISA modifications. - * More info and useful utilities to work with SBNI12 cards you can find - * at http://www.granch.com (English) or http://www.granch.ru (Russian) - * - * This software may be used and distributed according to the terms - * of the GNU General Public License. - * - * - * 5.0.1 Jun 22 2001 - * - Fixed bug in probe - * 5.0.0 Jun 06 2001 - * - Driver was completely redesigned by Denis I.Timofeev, - * -
[PATCH 2/2] isdn: capi: dead code removal
The staging isdn drivers are gone, and CONFIG_BT_CMTP is now the only user. This means a lot of the code in the subsystem has no remaining callers and can be removed. Change the capi user space front-end to be part of kernelcapi, and the combined module to only be compiled if BT_CMTP is also enabled, then remove the interfaces that have no remaining callers. As the notifier list and the capi_drivers list have no callers outside of kcapi.c, the implementation gets much simpler. Some definitions from the include/linux/*.h headers are only needed internally and are moved to kcapi.h. Signed-off-by: Arnd Bergmann --- Documentation/isdn/interface_capi.rst | 71 drivers/isdn/Makefile | 2 +- drivers/isdn/capi/Kconfig | 32 +- drivers/isdn/capi/Makefile| 18 +- drivers/isdn/capi/capi.c | 14 +- drivers/isdn/capi/capilib.c | 202 drivers/isdn/capi/capiutil.c | 231 + drivers/isdn/capi/kcapi.c | 409 +-- drivers/isdn/capi/kcapi.h | 149 - drivers/isdn/capi/kcapi_proc.c| 34 +- include/linux/isdn/capilli.h | 18 - include/linux/isdn/capiutil.h | 456 -- include/linux/kernelcapi.h| 75 - include/uapi/linux/b1lli.h| 74 - 14 files changed, 179 insertions(+), 1606 deletions(-) delete mode 100644 drivers/isdn/capi/capilib.c delete mode 100644 include/uapi/linux/b1lli.h diff --git a/Documentation/isdn/interface_capi.rst b/Documentation/isdn/interface_capi.rst index 01a4b5ade9a4..fe2421444b76 100644 --- a/Documentation/isdn/interface_capi.rst +++ b/Documentation/isdn/interface_capi.rst @@ -26,13 +26,6 @@ This standard is freely available from https://www.capi.org. 2. Driver and Device Registration = -CAPI drivers optionally register themselves with Kernel CAPI by calling the -Kernel CAPI function register_capi_driver() with a pointer to a struct -capi_driver. This structure must be filled with the name and revision of the -driver, and optionally a pointer to a callback function, add_card(). The -registration can be revoked by calling the function unregister_capi_driver() -with a pointer to the same struct capi_driver. - CAPI drivers must register each of the ISDN devices they control with Kernel CAPI by calling the Kernel CAPI function attach_capi_ctr() with a pointer to a struct capi_ctr before they can be used. This structure must be filled with @@ -89,9 +82,6 @@ register_capi_driver(): the name of the driver, as a zero-terminated ASCII string ``char revision[32]`` the revision number of the driver, as a zero-terminated ASCII string -``int (*add_card)(struct capi_driver *driver, capicardparams *data)`` - a callback function pointer (may be NULL) - 4.2 struct capi_ctr --- @@ -178,12 +168,6 @@ to be set by the driver before calling attach_capi_ctr(): pointer to a callback function returning the entry for the device in the CAPI controller info table, /proc/capi/controller -``const struct file_operations *proc_fops`` - pointers to callback functions for the device's proc file - system entry, /proc/capi/controllers/; pointer to the device's - capi_ctr structure is available from struct proc_dir_entry::data - which is available from struct inode. - Note: Callback functions except send_message() are never called in interrupt context. @@ -267,25 +251,10 @@ _cmstruct alternative representation for CAPI parameters of type 'struct' _cmsg structure members. === = -Functions capi_cmsg2message() and capi_message2cmsg() are provided to convert -messages between their transport encoding described in the CAPI 2.0 standard -and their _cmsg structure representation. Note that capi_cmsg2message() does -not know or check the size of its destination buffer. The caller must make -sure it is big enough to accommodate the resulting CAPI message. - 5. Lower Layer Interface Functions == -(declared in ) - -:: - - void register_capi_driver(struct capi_driver *drvr) - void unregister_capi_driver(struct capi_driver *drvr) - -register/unregister a driver with Kernel CAPI - :: int attach_capi_ctr(struct capi_ctr *ctrlr) @@ -300,13 +269,6 @@ register/unregister a device (controller) with Kernel CAPI signal controller ready/not ready -:: - - void capi_ctr_suspend_output(struct capi_ctr *ctrlr) - void capi_ctr_resume_output(struct capi_ctr *ctrlr) - -signal suspend/resume - :: void capi_ctr_handle_message(struct capi_ctr * ctrlr, u16 applid, @@ -319,21 +281,6 @@ for forwarding to the specified application 6. Helper Functions and Macros == -Library functions (from ): - -:: - - v
[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
[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
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 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; - }
[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 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
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
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
[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); +
[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 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 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/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] 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
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
[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: [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
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 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 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,
[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
[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 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] 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] 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] [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] [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
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
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 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