| Message ID | 20210330131646.1401838-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | selinux: fix changing booleans | expand |
On Tue, Mar 30, 2021 at 9:16 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > This series contains a patch that fixes broken conditional AV list > duplication introduced by c7c556f1e81b ("selinux: refactor changing > booleans") and a couple "and while I'm here..." cleanup patches on top. > > Ondrej Mosnacek (3): > selinux: fix cond_list corruption when changing booleans > selinux: simplify duplicate_policydb_cond_list() by using kmemdup() > selinux: constify some avtab function arguments Please don't resubmit, but in the future if you are submitting a patch (or two (or three ...)) which is potential -stable material (and so far I think 1/3 qualifies) don't submit it in a patchset with trivial cleanup patches. Adding cleanup patches to a patchset that adds a feature is okay, but fixes should generally stand by themselves.
On Wed, Mar 31, 2021 at 3:13 AM Paul Moore <paul@paul-moore.com> wrote: > On Tue, Mar 30, 2021 at 9:16 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > This series contains a patch that fixes broken conditional AV list > > duplication introduced by c7c556f1e81b ("selinux: refactor changing > > booleans") and a couple "and while I'm here..." cleanup patches on top. > > > > Ondrej Mosnacek (3): > > selinux: fix cond_list corruption when changing booleans > > selinux: simplify duplicate_policydb_cond_list() by using kmemdup() > > selinux: constify some avtab function arguments > > Please don't resubmit, but in the future if you are submitting a patch > (or two (or three ...)) which is potential -stable material (and so > far I think 1/3 qualifies) don't submit it in a patchset with trivial > cleanup patches. Adding cleanup patches to a patchset that adds a > feature is okay, but fixes should generally stand by themselves. Okay, but in this case the patches are sort of interdependent, so I didn't want to mess with a rebase and sending conflicting patches... I did move the bugfix patch to the bottom so that at worst you can pick it out and ask me to resubmit the rest some other way. Since I'll need to respin the bugfix patch, will it be better if I repost just the one patch?
On Wed, Mar 31, 2021 at 4:23 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > On Wed, Mar 31, 2021 at 3:13 AM Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Mar 30, 2021 at 9:16 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > This series contains a patch that fixes broken conditional AV list > > > duplication introduced by c7c556f1e81b ("selinux: refactor changing > > > booleans") and a couple "and while I'm here..." cleanup patches on top. > > > > > > Ondrej Mosnacek (3): > > > selinux: fix cond_list corruption when changing booleans > > > selinux: simplify duplicate_policydb_cond_list() by using kmemdup() > > > selinux: constify some avtab function arguments > > > > Please don't resubmit, but in the future if you are submitting a patch > > (or two (or three ...)) which is potential -stable material (and so > > far I think 1/3 qualifies) don't submit it in a patchset with trivial > > cleanup patches. Adding cleanup patches to a patchset that adds a > > feature is okay, but fixes should generally stand by themselves. > > Okay, but in this case the patches are sort of interdependent, so I > didn't want to mess with a rebase and sending conflicting patches... I > did move the bugfix patch to the bottom so that at worst you can pick > it out and ask me to resubmit the rest some other way. That's understandable, but it results in a patchset which is never going to be applied as a whole (the bug fix(es) would go to stable-X, the rest to next). It's not the end of the world, but it's nice when there is at least some hope that a patchset could be merged all at once. Moving forward I would suggest keeping the fixes separate and submitting the cleanups in a separate patchset with a note that the depend on the other patch(es). Depending on the particular patches I'll either just merge them and deal with the conflicts, or hold the cleanups until they can be applied without conflict. > Since I'll need to respin the bugfix patch, will it be better if I > repost just the one patch? Let's try the above suggestion and see how it works; if it turns out to be a mess, we can work on "patching" the process :)
This series contains a patch that fixes broken conditional AV list duplication introduced by c7c556f1e81b ("selinux: refactor changing booleans") and a couple "and while I'm here..." cleanup patches on top. Ondrej Mosnacek (3): selinux: fix cond_list corruption when changing booleans selinux: simplify duplicate_policydb_cond_list() by using kmemdup() selinux: constify some avtab function arguments security/selinux/ss/avtab.c | 118 +++++++++++------------------- security/selinux/ss/avtab.h | 18 +++-- security/selinux/ss/conditional.c | 26 ++++--- 3 files changed, 65 insertions(+), 97 deletions(-)