On Fri, Feb 07, 2025 at 12:03:23PM +0300, Dan Carpenter wrote:
> Hello Christian Brauner,
>
> Commit f8c6e8bd9ad5 ("statmount: allow to retrieve idmappings") from
> Feb 4, 2025 (linux-next), leads to the following Smatch static
> checker warning:
>
> fs/namespace.c:5468 statmount_string()
> error: uninitialized symbol 'offp'.
Oh right, that's after Miklos' changes. That is an annoying subtle
interaction between two branches. I'll fix that once vfs.fixes lands
upstream.
Thanks for the report.
>
> fs/namespace.c
> 5388 static int statmount_string(struct kstatmount *s, u64 flag)
> 5389 {
> 5390 int ret = 0;
> 5391 size_t kbufsize;
> 5392 struct seq_file *seq = &s->seq;
> 5393 struct statmount *sm = &s->sm;
> 5394 u32 start, *offp;
> 5395
> 5396 /* Reserve an empty string at the beginning for any unset offsets */
> 5397 if (!seq->count)
> 5398 seq_putc(seq, 0);
> 5399
> 5400 start = seq->count;
> 5401
> 5402 switch (flag) {
> 5403 case STATMOUNT_FS_TYPE:
> 5404 offp = &sm->fs_type;
> 5405 ret = statmount_fs_type(s, seq);
> 5406 break;
> 5407 case STATMOUNT_MNT_ROOT:
> 5408 offp = &sm->mnt_root;
> 5409 ret = statmount_mnt_root(s, seq);
> 5410 break;
> 5411 case STATMOUNT_MNT_POINT:
> 5412 offp = &sm->mnt_point;
> 5413 ret = statmount_mnt_point(s, seq);
> 5414 break;
> 5415 case STATMOUNT_MNT_OPTS:
> 5416 offp = &sm->mnt_opts;
> 5417 ret = statmount_mnt_opts(s, seq);
> 5418 break;
> 5419 case STATMOUNT_OPT_ARRAY:
> 5420 offp = &sm->opt_array;
> 5421 ret = statmount_opt_array(s, seq);
> 5422 break;
> 5423 case STATMOUNT_OPT_SEC_ARRAY:
> 5424 offp = &sm->opt_sec_array;
> 5425 ret = statmount_opt_sec_array(s, seq);
> 5426 break;
> 5427 case STATMOUNT_FS_SUBTYPE:
> 5428 offp = &sm->fs_subtype;
> 5429 statmount_fs_subtype(s, seq);
> 5430 break;
> 5431 case STATMOUNT_SB_SOURCE:
> 5432 offp = &sm->sb_source;
> 5433 ret = statmount_sb_source(s, seq);
> 5434 break;
> 5435 case STATMOUNT_MNT_UIDMAP:
> 5436 sm->mnt_uidmap = start;
> 5437 ret = statmount_mnt_uidmap(s, seq);
>
> offp not initialized
>
> 5438 break;
> 5439 case STATMOUNT_MNT_GIDMAP:
> 5440 sm->mnt_gidmap = start;
> 5441 ret = statmount_mnt_gidmap(s, seq);
>
> Same here
>
> 5442 break;
> 5443 default:
> 5444 WARN_ON_ONCE(true);
> 5445 return -EINVAL;
> 5446 }
> 5447
> 5448 /*
> 5449 * If nothing was emitted, return to avoid setting the flag
> 5450 * and terminating the buffer.
> 5451 */
> 5452 if (seq->count == start)
> 5453 return ret;
> 5454 if (unlikely(check_add_overflow(sizeof(*sm), seq->count, &kbufsize)))
> 5455 return -EOVERFLOW;
> 5456 if (kbufsize >= s->bufsize)
> 5457 return -EOVERFLOW;
> 5458
> 5459 /* signal a retry */
> 5460 if (unlikely(seq_has_overflowed(seq)))
> 5461 return -EAGAIN;
> 5462
> 5463 if (ret)
> 5464 return ret;
> 5465
> 5466 seq->buf[seq->count++] = '\0';
> 5467 sm->mask |= flag;
> --> 5468 *offp = start;
> ^^^^^^^^^^^^^^
>
> 5469 return 0;
> 5470 }
>
> regards,
> dan carpenter
