On Fri, May 7, 2021 at 2:27 PM Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > On Fri, May 07, 2021 at 01:57:19PM +0200, Ondrej Mosnacek wrote: > > The commit that added this check did so in a very strange way - first > > security_locked_down() is called, its value stored into retval, and if > > it's nonzero, then an additional check is made for (change_irq || > > change_port), and if this is true, the function returns. However, if > > the goto exit branch is not taken, the code keeps the retval value and > > continues executing the function. Then, depending on whether > > uport->ops->verify_port is set, the retval value may or may not be reset > > to zero and eventually the error value from security_locked_down() may > > abort the function a few lines below. > > > > I will go out on a limb and assume that this isn't the intended behavior > > and that an error value from security_locked_down() was supposed to > > abort the function only in case (change_irq || change_port) is true. > > Are you _sure_ about this? > > Verification from the authors and users of this odd feature might be > good to have, as I am loath to change how this works without them > weighing in here. I'm not completely sure and I'm with you on not merging this without feedback from people involved in the original patch and/or whoever understands the logic in said function. -- Ondrej Mosnacek Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.
