Re: [PATCH v3] media: v4l2-ctrls: do nothing if controls[0] == NULL in cluster functions

[Hans Verkuil]

On 17/09/2026 05:37, Rokinthan p wrote:
> The V4L2 control framework is designed to allow drivers to instantiate
>     controls and clusters without checking for errors after each call,
>     checking hdl->error only once at the end.
> 
>     However, if allocating the master control (controls[0]) fails, e.g. due to
>     memory allocation failure, v4l2_ctrl_cluster() triggers a WARNING:
>       ncontrols == 0 || controls[0] == NULL
>       WARNING: drivers/media/v4l2-core/v4l2-ctrls-core.c:2525 at
>   v4l2_ctrl_cluster
> 
>     Additionally, v4l2_ctrl_auto_cluster() attempts to dereference
>     master->minimum without checking if controls[0] is NULL, leading to a
>     NULL pointer dereference when master control creation fails.
> 
>     Update both v4l2_ctrl_cluster() and v4l2_ctrl_auto_cluster() to silently
>     return if controls[0] is NULL, preserving the design that control
>     creation errors are caught at the end when the driver checks hdl->error.
>     Also update function documentation in include/media/v4l2-ctrls.h.
> 
>     Fixes: 71c689dc2e73 ("media: v4l2-ctrls: split up into four source files")
>     Reported-by: syzbot+cf896de36144391bcde1@xxxxxxxxxxxxxxxxxxxxxxxxx
>     Closes: https://syzkaller.appspot.com/bug?extid=cf896de36144391bcde1
>     Suggested-by: Hans Verkuil <hverkuil@xxxxxxxxxx>
>     Signed-off-by: Rohinthan <rokinthanp03@xxxxxxxxx>

This says Rohinthan, but your email says 'Rokinthan p', what is correct?
Also, why do you have a 'Reply-To: www.rokinthanp03@xxxxxxxxx' field? That's
not going to work.

Regards,

	Hans

>     ---
>     v2 -> v3:
>     - Fix the issue in the V4L2 core (v4l2-ctrls-core.c) instead of hackrf,
>       as suggested by Hans Verkuil.
>     - Return early if controls[0] == NULL in both v4l2_ctrl_cluster() and
>       v4l2_ctrl_auto_cluster().
>     - Update function documentation in include/media/v4l2-ctrls.h.
>     - Revert hackrf driver changes.
> 
>     v1 -> v2:
>     - Corrected commit hash in Fixes tag.
> 
>      drivers/media/v4l2-core/v4l2-ctrls-core.c | 16 ++++++++++++----
>      include/media/v4l2-ctrls.h                |  4 ++++
>      2 files changed, 16 insertions(+), 4 deletions(-)
> 
>     diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-
>   core/v4l2-ctrls-core.c
>     --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
>     +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
>     @@ -2490,7 +2490,10 @@ void v4l2_ctrl_cluster(unsigned ncontrols, struct
>   v4l2_ctrl **controls)
>          int i;
> 
>          /* The first control is the master control and it must not be
> NULL */
>     -    if (WARN_ON(ncontrols == 0 || controls[0] == NULL))
>     +    if (WARN_ON(ncontrols == 0))
>     +        return;
>     +
>     +    if (!controls[0])
>              return;
> 
>          for (i = 0; i < ncontrols; i++) {
>     @@ -2508,12 +2511,17 @@ EXPORT_SYMBOL(v4l2_ctrl_cluster);
>      void v4l2_ctrl_auto_cluster(unsigned ncontrols, struct v4l2_ctrl
> **controls,
>                      u8 manual_val, bool set_volatile)
>      {
>     -    struct v4l2_ctrl *master = controls[0];
>     +    struct v4l2_ctrl *master;
>          u32 flag = 0;
>          int i;
> 
>     +    if (WARN_ON(ncontrols <= 1))
>     +        return;
>     +
>     +    if (!controls[0])
>     +        return;
>     +
>     +    master = controls[0];
>          v4l2_ctrl_cluster(ncontrols, controls);
>     -    WARN_ON(ncontrols <= 1);
>          WARN_ON(manual_val < master->minimum || manual_val > master->maximum);
>          WARN_ON(set_volatile && !has_op(master, g_volatile_ctrl));
>          master->is_auto = true;
>     diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
>     --- a/include/media/v4l2-ctrls.h
>     +++ b/include/media/v4l2-ctrls.h
>     @@ -834,6 +834,8 @@ struct v4l2_ctrl_config {
>       *
>       * @ncontrols:    The number of controls in this cluster.
>       * @controls:    The cluster control array of size @ncontrols.
>     + *
>     + * If controls[0] is NULL, then this function does nothing and
> just returns.
>       */
>      void v4l2_ctrl_cluster(unsigned int ncontrols, struct v4l2_ctrl
> **controls);
> 
>     @@ -868,6 +870,8 @@ void v4l2_ctrl_cluster(unsigned int ncontrols, struct
>   v4l2_ctrl **controls);
>       * In addition, this function will set the %V4L2_CTRL_FLAG_UPDATE flag
>       * on the autofoo control and %V4L2_CTRL_FLAG_INACTIVE on the foo
>   control(s)
>       * if autofoo is in auto mode.
>     + *
>     + * If controls[0] is NULL, then this function does nothing and
> just returns.
>       */
>      void v4l2_ctrl_auto_cluster(unsigned int ncontrols,
>                      struct v4l2_ctrl **controls,