* [PATCH v2 0/3] staging: most: replace BUG_ON() with proper error handling
@ 2025-10-21 13:09 Olle Lukowski
2025-10-21 13:09 ` [PATCH v2 1/3] staging: most: i2c: replace BUG_ON() with proper checks and error returns Olle Lukowski
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Olle Lukowski @ 2025-10-21 13:09 UTC (permalink / raw)
To: Parthiban Veerasooran, Christian Gromm, Greg Kroah-Hartman
Cc: linux-staging, linux-kernel, Olle Lukowski
This small series replaces BUG_ON() calls in three staging MOST drivers
with appropriate error handling.
The affected drivers are:
- drivers/staging/most/i2c/i2c.c
- drivers/staging/most/dim2/dim2.c
- drivers/staging/most/video/video.c
All changes are mechanical and do not alter runtime behavior, except that
the kernel won't panic if a driver invariant is violated.
Tested with checkpatch.pl and compile-tested (make M=drivers/staging/most).
Signed-off-by: Olle Lukowski <olle@lukowski.dev>
---
Changes in v2:
- Drop WARN_ON_ONCE() per review (Greg KH). Many systems enable
panic_on_warn; use plain checks + error returns instead.
- Link to v1: https://lore.kernel.org/r/20251021-staging-most-warn-v1-0-4cdd3745bbdc@lukowski.dev
---
Olle Lukowski (3):
staging: most: i2c: replace BUG_ON() with proper checks and error returns
staging: most: dim2: replace BUG_ON() with proper checks and error returns
staging: most: video: replace BUG_ON() with proper check
drivers/staging/most/dim2/dim2.c | 27 +++++++++++++++++++--------
drivers/staging/most/i2c/i2c.c | 9 ++++++---
drivers/staging/most/video/video.c | 3 ++-
3 files changed, 27 insertions(+), 12 deletions(-)
---
base-commit: 211ddde0823f1442e4ad052a2f30f050145ccada
change-id: 20251021-staging-most-warn-51deb2e454aa
Best regards,
--
Olle Lukowski <olle@lukowski.dev>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/3] staging: most: i2c: replace BUG_ON() with proper checks and error returns 2025-10-21 13:09 [PATCH v2 0/3] staging: most: replace BUG_ON() with proper error handling Olle Lukowski @ 2025-10-21 13:09 ` Olle Lukowski 2025-10-21 13:09 ` [PATCH v2 2/3] staging: most: dim2: " Olle Lukowski 2025-10-21 13:09 ` [PATCH v2 3/3] staging: most: video: replace BUG_ON() with proper check Olle Lukowski 2 siblings, 0 replies; 6+ messages in thread From: Olle Lukowski @ 2025-10-21 13:09 UTC (permalink / raw) To: Parthiban Veerasooran, Christian Gromm, Greg Kroah-Hartman Cc: linux-staging, linux-kernel, Olle Lukowski Replace BUG_ON() checks for invalid channel indices with proper checks and return -EINVAL to avoid crashing the kernel unnecessarily. Signed-off-by: Olle Lukowski <olle@lukowski.dev> --- drivers/staging/most/i2c/i2c.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c index 184b2dd11..db806b00d 100644 --- a/drivers/staging/most/i2c/i2c.c +++ b/drivers/staging/most/i2c/i2c.c @@ -71,7 +71,8 @@ static int configure_channel(struct most_interface *most_iface, struct hdm_i2c *dev = to_hdm(most_iface); unsigned int delay, pr; - BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS); + if (ch_idx < 0 || ch_idx >= NUM_CHANNELS) + return -EINVAL; if (channel_config->data_type != MOST_CH_CONTROL) { pr_err("bad data type for channel %d\n", ch_idx); @@ -125,7 +126,8 @@ static int enqueue(struct most_interface *most_iface, struct hdm_i2c *dev = to_hdm(most_iface); int ret; - BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS); + if (ch_idx < 0 || ch_idx >= NUM_CHANNELS) + return -EINVAL; if (ch_idx == CH_RX) { /* RX */ @@ -170,7 +172,8 @@ static int poison_channel(struct most_interface *most_iface, struct hdm_i2c *dev = to_hdm(most_iface); struct mbo *mbo; - BUG_ON(ch_idx < 0 || ch_idx >= NUM_CHANNELS); + if (ch_idx < 0 || ch_idx >= NUM_CHANNELS) + return -EINVAL; if (ch_idx == CH_RX) { if (!polling_rate) -- 2.51.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] staging: most: dim2: replace BUG_ON() with proper checks and error returns 2025-10-21 13:09 [PATCH v2 0/3] staging: most: replace BUG_ON() with proper error handling Olle Lukowski 2025-10-21 13:09 ` [PATCH v2 1/3] staging: most: i2c: replace BUG_ON() with proper checks and error returns Olle Lukowski @ 2025-10-21 13:09 ` Olle Lukowski 2025-10-21 16:02 ` Greg Kroah-Hartman 2025-10-21 13:09 ` [PATCH v2 3/3] staging: most: video: replace BUG_ON() with proper check Olle Lukowski 2 siblings, 1 reply; 6+ messages in thread From: Olle Lukowski @ 2025-10-21 13:09 UTC (permalink / raw) To: Parthiban Veerasooran, Christian Gromm, Greg Kroah-Hartman Cc: linux-staging, linux-kernel, Olle Lukowski Replace BUG_ON() calls with proper checks to prevent unnecessary kernel panics. Return appropriate error codes (-EINVAL or -EFAULT) instead of crashing the system. Signed-off-by: Olle Lukowski <olle@lukowski.dev> --- drivers/staging/most/dim2/dim2.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c index dad2abe6c..d0832704b 100644 --- a/drivers/staging/most/dim2/dim2.c +++ b/drivers/staging/most/dim2/dim2.c @@ -166,8 +166,10 @@ static int try_start_dim_transfer(struct hdm_channel *hdm_ch) unsigned long flags; struct dim_ch_state st; - BUG_ON(!hdm_ch); - BUG_ON(!hdm_ch->is_initialized); + if (!hdm_ch) + return -EINVAL; + if (!hdm_ch->is_initialized) + return -EINVAL; spin_lock_irqsave(&dim_lock, flags); if (list_empty(head)) { @@ -188,7 +190,11 @@ static int try_start_dim_transfer(struct hdm_channel *hdm_ch) return -EAGAIN; } - BUG_ON(mbo->bus_address == 0); + if (mbo->bus_address == 0) { + spin_unlock_irqrestore(&dim_lock, flags); + return -EFAULT; + } + if (!dim_enqueue_buffer(&hdm_ch->ch, mbo->bus_address, buf_size)) { list_del(head->next); spin_unlock_irqrestore(&dim_lock, flags); @@ -269,8 +275,10 @@ static void service_done_flag(struct dim2_hdm *dev, int ch_idx) unsigned long flags; u8 *data; - BUG_ON(!hdm_ch); - BUG_ON(!hdm_ch->is_initialized); + if (!hdm_ch) + return; + if (!hdm_ch->is_initialized) + return; spin_lock_irqsave(&dim_lock, flags); @@ -455,7 +463,8 @@ static int configure_channel(struct most_interface *most_iface, int ch_idx, int const ch_addr = ch_idx * 2 + 2; struct hdm_channel *const hdm_ch = dev->hch + ch_idx; - BUG_ON(ch_idx < 0 || ch_idx >= DMA_CHANNELS); + if (ch_idx < 0 || ch_idx >= DMA_CHANNELS) + return -EINVAL; if (hdm_ch->is_initialized) return -EPERM; @@ -567,7 +576,8 @@ static int enqueue(struct most_interface *most_iface, int ch_idx, struct hdm_channel *hdm_ch = dev->hch + ch_idx; unsigned long flags; - BUG_ON(ch_idx < 0 || ch_idx >= DMA_CHANNELS); + if (ch_idx < 0 || ch_idx >= DMA_CHANNELS) + return -EINVAL; if (!hdm_ch->is_initialized) return -EPERM; @@ -643,7 +653,8 @@ static int poison_channel(struct most_interface *most_iface, int ch_idx) u8 hal_ret; int ret = 0; - BUG_ON(ch_idx < 0 || ch_idx >= DMA_CHANNELS); + if (ch_idx < 0 || ch_idx >= DMA_CHANNELS) + return -EINVAL; if (!hdm_ch->is_initialized) return -EPERM; -- 2.51.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/3] staging: most: dim2: replace BUG_ON() with proper checks and error returns 2025-10-21 13:09 ` [PATCH v2 2/3] staging: most: dim2: " Olle Lukowski @ 2025-10-21 16:02 ` Greg Kroah-Hartman 0 siblings, 0 replies; 6+ messages in thread From: Greg Kroah-Hartman @ 2025-10-21 16:02 UTC (permalink / raw) To: Olle Lukowski Cc: Parthiban Veerasooran, Christian Gromm, linux-staging, linux-kernel On Tue, Oct 21, 2025 at 04:09:29PM +0300, Olle Lukowski wrote: > Replace BUG_ON() calls with proper checks to prevent unnecessary kernel > panics. Return appropriate error codes (-EINVAL or -EFAULT) instead of > crashing the system. > > Signed-off-by: Olle Lukowski <olle@lukowski.dev> > --- > drivers/staging/most/dim2/dim2.c | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c > index dad2abe6c..d0832704b 100644 > --- a/drivers/staging/most/dim2/dim2.c > +++ b/drivers/staging/most/dim2/dim2.c > @@ -166,8 +166,10 @@ static int try_start_dim_transfer(struct hdm_channel *hdm_ch) > unsigned long flags; > struct dim_ch_state st; > > - BUG_ON(!hdm_ch); > - BUG_ON(!hdm_ch->is_initialized); > + if (!hdm_ch) > + return -EINVAL; > + if (!hdm_ch->is_initialized) > + return -EINVAL; Can these things ever actually happen? In looking at the code, I don't see how that could be, do you? Let's not check for things that are impossible to ever hit. Same with the other changes in this series, please verify that these are actually possible to happen. If not, then just remove the check. > spin_lock_irqsave(&dim_lock, flags); > if (list_empty(head)) { > @@ -188,7 +190,11 @@ static int try_start_dim_transfer(struct hdm_channel *hdm_ch) > return -EAGAIN; > } > > - BUG_ON(mbo->bus_address == 0); > + if (mbo->bus_address == 0) { > + spin_unlock_irqrestore(&dim_lock, flags); > + return -EFAULT; You need to do more than just that here :( Please be very careful on error paths to properly clean up everything. thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] staging: most: video: replace BUG_ON() with proper check 2025-10-21 13:09 [PATCH v2 0/3] staging: most: replace BUG_ON() with proper error handling Olle Lukowski 2025-10-21 13:09 ` [PATCH v2 1/3] staging: most: i2c: replace BUG_ON() with proper checks and error returns Olle Lukowski 2025-10-21 13:09 ` [PATCH v2 2/3] staging: most: dim2: " Olle Lukowski @ 2025-10-21 13:09 ` Olle Lukowski 2025-10-21 16:02 ` Greg Kroah-Hartman 2 siblings, 1 reply; 6+ messages in thread From: Olle Lukowski @ 2025-10-21 13:09 UTC (permalink / raw) To: Parthiban Veerasooran, Christian Gromm, Greg Kroah-Hartman Cc: linux-staging, linux-kernel, Olle Lukowski Replace a BUG_ON() call with a proper check to prevent an unnecessary kernel panic. Signed-off-by: Olle Lukowski <olle@lukowski.dev> --- drivers/staging/most/video/video.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c index 32f71d9a9..3a7ee2791 100644 --- a/drivers/staging/most/video/video.c +++ b/drivers/staging/most/video/video.c @@ -575,7 +575,8 @@ static void __exit comp_exit(void) most_deregister_configfs_subsys(&comp); most_deregister_component(&comp); - BUG_ON(!list_empty(&video_devices)); + if (!list_empty(&video_devices)) + pr_err("video_devices list not empty during exit\n"); } module_init(comp_init); -- 2.51.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] staging: most: video: replace BUG_ON() with proper check 2025-10-21 13:09 ` [PATCH v2 3/3] staging: most: video: replace BUG_ON() with proper check Olle Lukowski @ 2025-10-21 16:02 ` Greg Kroah-Hartman 0 siblings, 0 replies; 6+ messages in thread From: Greg Kroah-Hartman @ 2025-10-21 16:02 UTC (permalink / raw) To: Olle Lukowski Cc: Parthiban Veerasooran, Christian Gromm, linux-staging, linux-kernel On Tue, Oct 21, 2025 at 04:09:30PM +0300, Olle Lukowski wrote: > Replace a BUG_ON() call with a proper check to prevent an unnecessary > kernel panic. > > Signed-off-by: Olle Lukowski <olle@lukowski.dev> > --- > drivers/staging/most/video/video.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c > index 32f71d9a9..3a7ee2791 100644 > --- a/drivers/staging/most/video/video.c > +++ b/drivers/staging/most/video/video.c > @@ -575,7 +575,8 @@ static void __exit comp_exit(void) > > most_deregister_configfs_subsys(&comp); > most_deregister_component(&comp); > - BUG_ON(!list_empty(&video_devices)); > + if (!list_empty(&video_devices)) > + pr_err("video_devices list not empty during exit\n"); Is this possible to ever have happen? thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-10-21 16:02 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-21 13:09 [PATCH v2 0/3] staging: most: replace BUG_ON() with proper error handling Olle Lukowski 2025-10-21 13:09 ` [PATCH v2 1/3] staging: most: i2c: replace BUG_ON() with proper checks and error returns Olle Lukowski 2025-10-21 13:09 ` [PATCH v2 2/3] staging: most: dim2: " Olle Lukowski 2025-10-21 16:02 ` Greg Kroah-Hartman 2025-10-21 13:09 ` [PATCH v2 3/3] staging: most: video: replace BUG_ON() with proper check Olle Lukowski 2025-10-21 16:02 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).