All of lore.kernel.org
 help / color / mirror / Atom feed
* Thoughts on 'btrfs device stats' and security.
@ 2017-03-17 18:25 Austin S. Hemmelgarn
  2017-03-17 19:01 ` Eric Sandeen
  0 siblings, 1 reply; 3+ messages in thread
From: Austin S. Hemmelgarn @ 2017-03-17 18:25 UTC (permalink / raw)
  To: Btrfs BTRFS

I'm currently working on a plugin for colllectd [1] to track per-device 
per-filesystem error rates for BTRFS volumes.  Overall, this is actually 
going quite well (I've got most of the secondary logic like matching 
filesystems to watch and parsing the data done already), but I've come 
across a rather nasty caveat on the actual data collection part.

As of right now, there are only two ways I can see to get this data:
1. Parse the output of `btrfs device stats` for the filesystem.
2. Make the same ioctl() call that `btrfs device stats` does and compose 
the data yourself.

In both cases, one of the following has to be the case:
1. You're running as root.
2. You're running SUID root.
3. You're running with CAP_SYS_ADMIN (I'm not 100% certain that this is 
the correct capability, but it appears to be the case from my testing).

In other words, you have to reduce the overall security of your system 
to be able to get this data which is itself not security sensitive for 
most intents and purposes.

Looking at this from the perspective of actual data collection, there 
are a handful of things that come to mind:
* You already know either device names or device-id's, because the ioctl 
is called per-device and uses one of those to identify the device to 
return data for (I'm not sure from reading the code which), so this 
aspect is by definition not sensitive data.
* While there are theoretically attacks that this data could be useful 
for, the only ones I can think of are largely pointless because they 
require write access to the underlying storage and provide information 
which would by definition already be available by other (more efficient 
and less obtrusive) means if you had such access.
* This is somewhat useful data for an admin who may not have root access 
to retrieve.
* The use of an ioctl makes it non-trivial to access this data.
* We already expose allocation data through /sys/fs/btrfs (in the 
per-filesystem `allocation` directory).

On this note, I'd like to propose that the error counters be exposed in 
some way through sysfs so you don't have to parse the output of `btrfs 
device stats` (and incur the fork()+exec() overhead) or fight with 
ioctls.  I would offer to work on this myself, but I have exactly zero 
experience with the kernel side of sysfs, very limited experience with 
kernel coding in general, and I'm not likely to have much (if any) time 
to work on this for quite a while.

[1] https://collectd.org/

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Thoughts on 'btrfs device stats' and security.
  2017-03-17 18:25 Thoughts on 'btrfs device stats' and security Austin S. Hemmelgarn
@ 2017-03-17 19:01 ` Eric Sandeen
  2017-03-17 19:45   ` Austin S. Hemmelgarn
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Sandeen @ 2017-03-17 19:01 UTC (permalink / raw)
  To: Austin S. Hemmelgarn, Btrfs BTRFS

On 3/17/17 11:25 AM, Austin S. Hemmelgarn wrote:
> I'm currently working on a plugin for colllectd [1] to track per-device per-filesystem error rates for BTRFS volumes.  Overall, this is actually going quite well (I've got most of the secondary logic like matching filesystems to watch and parsing the data done already), but I've come across a rather nasty caveat on the actual data collection part.
> 
> As of right now, there are only two ways I can see to get this data:
> 1. Parse the output of `btrfs device stats` for the filesystem.
> 2. Make the same ioctl() call that `btrfs device stats` does and compose the data yourself.
> 
> In both cases, one of the following has to be the case:
> 1. You're running as root.
> 2. You're running SUID root.
> 3. You're running with CAP_SYS_ADMIN (I'm not 100% certain that this is the correct capability, but it appears to be the case from my testing).
> 
> In other words, you have to reduce the overall security of your system to be able to get this data which is itself not security sensitive for most intents and purposes.

As one datapoint, xfs stats are ugo+r -

see /proc/fs/xfs/stat or /sys/fs/xfs/<device>/stats/stats

-r--r--r--. 1 root root 4096 Mar 17 13:58 stats

However, the stats_clear file is only writable by root

--w-------. 1 root root 4096 Mar 17 13:58 stats_clear

Stats & other info for ext4 are also ugo+r, other than
an error trigger which is only writable by root, and
for which a read is meaningless.

/sys/fs/ext4/sda1/

-r--r--r--. 1 root root 4096 Mar 17 14:00 delayed_allocation_blocks
-r--r--r--. 1 root root 4096 Mar 17 14:00 errors_count
-rw-r--r--. 1 root root 4096 Mar 17 14:00 err_ratelimit_burst
-rw-r--r--. 1 root root 4096 Mar 17 14:00 err_ratelimit_interval_ms
-rw-r--r--. 1 root root 4096 Mar 17 14:00 extent_max_zeroout_kb
-r--r--r--. 1 root root 4096 Mar 17 14:00 first_error_time
-rw-r--r--. 1 root root 4096 Mar 17 14:00 inode_goal
-rw-r--r--. 1 root root 4096 Mar 17 14:00 inode_readahead_blks
-r--r--r--. 1 root root 4096 Mar 17 14:00 last_error_time
-r--r--r--. 1 root root 4096 Mar 17 14:00 lifetime_write_kbytes
-r--r--r--. 1 root root 4096 Mar 17 14:00 max_writeback_mb_bump
-rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_group_prealloc
-rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_max_to_scan
-rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_min_to_scan
-rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_order2_req
-rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_stats
-rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_stream_req
-rw-r--r--. 1 root root 4096 Mar 17 14:00 msg_ratelimit_burst
-rw-r--r--. 1 root root 4096 Mar 17 14:00 msg_ratelimit_interval_ms
-rw-r--r--. 1 root root 4096 Mar 17 14:00 reserved_clusters
-r--r--r--. 1 root root 4096 Mar 17 14:00 session_write_kbytes
--w-------. 1 root root 4096 Mar 17 14:00 trigger_fs_error
-rw-r--r--. 1 root root 4096 Mar 17 14:00 warning_ratelimit_burst
-rw-r--r--. 1 root root 4096 Mar 17 14:00 warning_ratelimit_interval_ms


-Eric

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Thoughts on 'btrfs device stats' and security.
  2017-03-17 19:01 ` Eric Sandeen
@ 2017-03-17 19:45   ` Austin S. Hemmelgarn
  0 siblings, 0 replies; 3+ messages in thread
From: Austin S. Hemmelgarn @ 2017-03-17 19:45 UTC (permalink / raw)
  To: Eric Sandeen, Btrfs BTRFS

On 2017-03-17 15:01, Eric Sandeen wrote:
> On 3/17/17 11:25 AM, Austin S. Hemmelgarn wrote:
>> I'm currently working on a plugin for colllectd [1] to track per-device per-filesystem error rates for BTRFS volumes.  Overall, this is actually going quite well (I've got most of the secondary logic like matching filesystems to watch and parsing the data done already), but I've come across a rather nasty caveat on the actual data collection part.
>>
>> As of right now, there are only two ways I can see to get this data:
>> 1. Parse the output of `btrfs device stats` for the filesystem.
>> 2. Make the same ioctl() call that `btrfs device stats` does and compose the data yourself.
>>
>> In both cases, one of the following has to be the case:
>> 1. You're running as root.
>> 2. You're running SUID root.
>> 3. You're running with CAP_SYS_ADMIN (I'm not 100% certain that this is the correct capability, but it appears to be the case from my testing).
>>
>> In other words, you have to reduce the overall security of your system to be able to get this data which is itself not security sensitive for most intents and purposes.
>
> As one datapoint, xfs stats are ugo+r -
>
> see /proc/fs/xfs/stat or /sys/fs/xfs/<device>/stats/stats
>
> -r--r--r--. 1 root root 4096 Mar 17 13:58 stats
>
> However, the stats_clear file is only writable by root
>
> --w-------. 1 root root 4096 Mar 17 13:58 stats_clear
That pretty much matches what I was thinking, albeit having one data 
file and one clear file for each device in each filesystem since the 
error counters are per-device per-filesystem, and there are multiple 
reasons to reset the counters on only one (device, filesystem) pair at a 
time.

On that note, it would be kind of nice to get some more extended 
performance stats like you can get from XFS and ext4, and sysfs is 
probably the best place for those to go too, but that's obviously not as 
important as the error counters being easily accessible.
>
> Stats & other info for ext4 are also ugo+r, other than
> an error trigger which is only writable by root, and
> for which a read is meaningless.
>
> /sys/fs/ext4/sda1/
>
> -r--r--r--. 1 root root 4096 Mar 17 14:00 delayed_allocation_blocks
> -r--r--r--. 1 root root 4096 Mar 17 14:00 errors_count
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 err_ratelimit_burst
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 err_ratelimit_interval_ms
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 extent_max_zeroout_kb
> -r--r--r--. 1 root root 4096 Mar 17 14:00 first_error_time
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 inode_goal
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 inode_readahead_blks
> -r--r--r--. 1 root root 4096 Mar 17 14:00 last_error_time
> -r--r--r--. 1 root root 4096 Mar 17 14:00 lifetime_write_kbytes
> -r--r--r--. 1 root root 4096 Mar 17 14:00 max_writeback_mb_bump
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_group_prealloc
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_max_to_scan
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_min_to_scan
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_order2_req
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_stats
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_stream_req
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 msg_ratelimit_burst
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 msg_ratelimit_interval_ms
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 reserved_clusters
> -r--r--r--. 1 root root 4096 Mar 17 14:00 session_write_kbytes
> --w-------. 1 root root 4096 Mar 17 14:00 trigger_fs_error
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 warning_ratelimit_burst
> -rw-r--r--. 1 root root 4096 Mar 17 14:00 warning_ratelimit_interval_ms
>
>
> -Eric
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-03-18  5:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-17 18:25 Thoughts on 'btrfs device stats' and security Austin S. Hemmelgarn
2017-03-17 19:01 ` Eric Sandeen
2017-03-17 19:45   ` Austin S. Hemmelgarn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.