linux-kernel - Re: [bug report] firewire: add kernel API to access CYCLE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Tue, 12 Apr 2022 20:22:04 +0900
From:   Takashi Sakamoto <o-takashi@...amocchi.jp>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     alsa-devel@...a-project.org, linux1394-devel@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org
Subject: Re: [bug report] firewire: add kernel API to access CYCLE_TIME
 register

Hi,

On Mon, Apr 11, 2022 at 10:01:25AM +0300, Dan Carpenter wrote:
> Hello Takashi Sakamoto,
> 
> The patch baa914cd81f5: "firewire: add kernel API to access
> CYCLE_TIME register" from Apr 5, 2022, leads to the following Smatch
> static checker warning:
> 
> 	drivers/firewire/core-cdev.c:1235 ioctl_get_cycle_timer2()
> 	error: uninitialized symbol 'cycle_time'.
> 
> drivers/firewire/core-cdev.c
>     1209 static int ioctl_get_cycle_timer2(struct client *client, union ioctl_arg *arg)
>     1210 {
>     1211         struct fw_cdev_get_cycle_timer2 *a = &arg->get_cycle_timer2;
>     1212         struct fw_card *card = client->device->card;
>     1213         struct timespec64 ts = {0, 0};
>     1214         u32 cycle_time;
>     1215         int ret = 0;
>     1216 
>     1217         local_irq_disable();
>     1218 
>     1219         ret = fw_card_read_cycle_time(card, &cycle_time);
>     1220         if (ret < 0)
>     1221                 goto end;
>                          ^^^^^^^^
> "cycle_time" not initialized on error path.
> 
>     1222 
>     1223         switch (a->clk_id) {
>     1224         case CLOCK_REALTIME:      ktime_get_real_ts64(&ts);        break;
>     1225         case CLOCK_MONOTONIC:     ktime_get_ts64(&ts);                break;
>     1226         case CLOCK_MONOTONIC_RAW: ktime_get_raw_ts64(&ts);        break;
>     1227         default:
>     1228                 ret = -EINVAL;
>     1229         }
>     1230 end:
>     1231         local_irq_enable();
>     1232 
>     1233         a->tv_sec      = ts.tv_sec;
>     1234         a->tv_nsec     = ts.tv_nsec;
> --> 1235         a->cycle_timer = cycle_time;
>     1236 
>     1237         return ret;
>     1238 }

Thanks for the report. Indeed, it leaks the unidentified value on kernel
stack to userspace. I'll post fix later.


Regards

Takashi Sakamoto