Luis Chamberlain <mcgrof@xxxxxxxxxx> writes: > On Mon, Nov 26, 2018 at 11:29:40PM -0600, Eric W. Biederman wrote: >> Luis Chamberlain <mcgrof@xxxxxxxxxx> writes: >> > Thanks for the description of how to run into the issue described but >> > is there also a practical use case today where this is happening? I ask >> > as it would be good to know the severity of the issue in the real world >> > today. >> >> People trying to run containers without a root user in the container. >> It atypical but something doable. > > My question was if there are generic tools / propreitary tools which are > doing this widely *today*. Or is this just a custom setup some folks > use? > >> We spoke about this at LPC. And this is the correct behavioral change. >> >> The problem is there is a default value for i_uid and i_gid that is >> correct in the general case. That default value is not corect for >> sysctl, because proc is weird. As the sysctl permission check in >> test_perm are all against GLOBAL_ROOT_UID and GLOBAL_ROOT_GID we did not >> notice that i_uid and i_gid were being set wrong. >> >> So all this patch does is fix the default values i_uid and i_gid. >> >> The commit comment seems worth cleaning up. But for the >> content of the code. > > The logic seems sensible then, but are we implicating what a container > does with its sysctl values onto the entire system? If so, sure, it > seems you want this for networking purposes as there are a series of > sysctl values a container may want to muck with, but are we sure we > want the same for *all* sysctl entries? No. Please look at the patch again. It sets the default uid and gid for sysctl entries to 0. AKA GLOBAL_ROOT_UID and GLOBAL_ROOT_GID because there is a bug and they were not set to that value. Those are the uids and gids that are tested agasint. It just happens you have to be in a weird configuration for this bug to become a problem. Eric