[bug report] drm: add DRM_SET_CLIENT_NAME ioctl

[Pierre-Eric Pelloux-Prayer]

Hi,

Le 10/10/2024 à 12:02, Dan Carpenter a écrit :
> Hello Pierre-Eric Pelloux-Prayer,
> 
> Commit 56c594d8df64 ("drm: add DRM_SET_CLIENT_NAME ioctl") from Oct
> 3, 2024 (linux-next), leads to the following Smatch static checker
> warning:
> 
> 	drivers/gpu/drm/drm_debugfs.c:104 drm_clients_info()
> 	warn: was precision intended? '64'
> 
> drivers/gpu/drm/drm_debugfs.c
>      73 static int drm_clients_info(struct seq_file *m, void *data)
>      74 {
>      75         struct drm_debugfs_entry *entry = m->private;
>      76         struct drm_device *dev = entry->dev;
>      77         struct drm_file *priv;
>      78         kuid_t uid;
>      79
>      80         seq_printf(m,
>      81                    "%20s %5s %3s master a %5s %10s %*s\n",
>                                                            ^^^
> this was probably intended to be %.*s

No. The intent is to right-align the string, similar to what is done for the other
string fields.
It could have been written %64s, but since DRM_CLIENT_NAME_MAX_LEN exist, I've used
the %*s syntax.

Pierre-Eric


> 
>      82                    "command",
>      83                    "tgid",
>      84                    "dev",
>      85                    "uid",
>      86                    "magic",
>      87                    DRM_CLIENT_NAME_MAX_LEN,
>      88                    "name");
>      89
>      90         /* dev->filelist is sorted youngest first, but we want to present
>      91          * oldest first (i.e. kernel, servers, clients), so walk backwardss.
>      92          */
>      93         mutex_lock(&dev->filelist_mutex);
>      94         list_for_each_entry_reverse(priv, &dev->filelist, lhead) {
>      95                 bool is_current_master = drm_is_current_master(priv);
>      96                 struct task_struct *task;
>      97                 struct pid *pid;
>      98
>      99                 mutex_lock(&priv->client_name_lock);
>      100                 rcu_read_lock(); /* Locks priv->pid and pid_task()->comm! */
>      101                 pid = rcu_dereference(priv->pid);
>      102                 task = pid_task(pid, PIDTYPE_TGID);
>      103                 uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
> --> 104                 seq_printf(m, "%20s %5d %3d   %c    %c %5d %10u %*s\n",
>                                                                           ^^
> And this as well.  But why bother?  We know these strings are NUL terminated.
> 
>      105                            task ? task->comm : "<unknown>",
>      106                            pid_vnr(pid),
>      107                            priv->minor->index,
>      108                            is_current_master ? 'y' : 'n',
>      109                            priv->authenticated ? 'y' : 'n',
>      110                            from_kuid_munged(seq_user_ns(m), uid),
>      111                            priv->magic,
>      112                            DRM_CLIENT_NAME_MAX_LEN,
>      113                            priv->client_name ? priv->client_name : "<unset>");
>      114                 rcu_read_unlock();
>      115                 mutex_unlock(&priv->client_name_lock);
>      116         }
>      117         mutex_unlock(&dev->filelist_mutex);
>      118         return 0;
>      119 }
> 
> regards,
> dan carpenter