Re: [PATCH] iio: backend: fix uninitialized data in debugfs
Nuno Sá <[email protected]> Tue, 9 Jun 2026 09:10:14 +0100
| Newsgroups | org.kernel.vger.kernel-janitors,org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <aifJtgLkM2m3KciV@nsa> |
On Mon, Jun 08, 2026 at 04:51:38PM -0500, Maxwell Doose wrote: > On Mon, 08 Jun 2026 21:31:14 +0100 > Nuno Sá <[email protected]> wrote: > > > On Fri, 2026-06-05 at 11:28 +0300, Andy Shevchenko wrote: > > > On Fri, Jun 05, 2026 at 09:12:38AM +0300, Dan Carpenter wrote: > > > > On Thu, Jun 04, 2026 at 05:55:08PM +0300, Andy Shevchenko wrote: > > > > > On Thu, Jun 04, 2026 at 01:42:11PM +0300, Dan Carpenter wrote: > > > > > > On Thu, Jun 04, 2026 at 01:38:50PM +0300, Dan Carpenter wrote: > > > > > > > 168 ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val); > > > > > > > ^^^ > > > > > > > Uninitialized variable. > > > > > > > > > > > > s/variable/data/. > > > > > > > > > > With what I asked in the previous reply and what you explained there > > > > > (thanks, btw!) I still think your patches are not fully correct. They > > > > > will require to atomically write all or nothing. If we want support > > > > > partial writes we need to go with that differently (reset ppos when > > > > > we got enough or more than enough data). > > > > > > > > Requiring writes to syfs and debugfs be atomic is pretty normal and > > > > works well in practice. These are very small writes. > > > > > > Perhaps. In any case your patch will break existing partial writes, right? > > > I'm still considering that resetting ppos is the right thing to do. Just > > > need to find where the best place is to do that. > > > > I think anyone doing partial writes on a debugfs interface like this one is very > > unlikely but it is a fair point, yes. But can't we be more relaxed on debugfs? No > > userspace app should be relying on debugfs in order to work (though I know that > > actually happens). > > > > Anyways, this is one of those interesting edge cases and easy enough to get wrong. I > > guess we should either: > > > > 1. Improve simple_write_to_buffer() docs; > > 2. Or come up with a new simple_write_once_to_buffer() helper? > > > > When you say "simple_write_once_to_buffer()" do you mean a wrapper > around simple_write_to_buffer() that also checks if *ppos is 0 and > returns -EINVAL if *ppos isn't 0? If so I can start writing that > function. Though I guess we probably want to ask Jonathan about it > first since obviously he'll have his own opinions. Yeah basically not allowing partial writes. Partial writes are also very tricky to handle in terms of concurrency (assuming userspace can do whatever they want). But I would not rush into writing the helper without understanding how people feel about it (at least the ones in this thread). Might not be super beneficial. - Nuno Sá > > -- > best regards, > max