| Message ID | 20230420215331.88326-2-junxiao.bi@oracle.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [V4,1/2] debugfs: provide a way for relay user bypass lockdown | expand |
Any IO folks can help review this patch? Paul needs a confirm from you that the information blktrace exporting to userspace through the relay files are safe, not leaking information that userspace shouldn't know in lockdown mode. Thanks, Junxiao. On 4/20/23 2:53 PM, Junxiao Bi wrote: > blktrace trace files are per-cpu relay files that are used by kernel to > export IO metadata(IO events, type, target disk, offset and len etc.) to > userspace, no data from IO itself will be exported. Bypass lockdown for > these files will make blktrace work in lockdown mode. > > Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com> > --- > kernel/trace/blktrace.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c > index d5d94510afd3..e1a9f8b7d710 100644 > --- a/kernel/trace/blktrace.c > +++ b/kernel/trace/blktrace.c > @@ -490,10 +490,16 @@ static struct dentry *blk_create_buf_file_callback(const char *filename, > &relay_file_operations); > } > > +static bool blk_bypass_lockdown(struct inode *inode) > +{ > + return true; > +} > + > static const struct rchan_callbacks blk_relay_callbacks = { > .subbuf_start = blk_subbuf_start_callback, > .create_buf_file = blk_create_buf_file_callback, > .remove_buf_file = blk_remove_buf_file_callback, > + .bypass_lockdown = blk_bypass_lockdown, > }; > > static void blk_trace_setup_lba(struct blk_trace *bt,
On 4/25/23 11:55?AM, Junxiao Bi wrote: > Any IO folks can help review this patch? > > Paul needs a confirm from you that the information blktrace exporting > to userspace through the relay files are safe, not leaking information > that userspace shouldn't know in lockdown mode. I don't know anything about what lockdown is, but in terms of blktrace, it is a way to trace meta data associated with IO. It'll tell you things like "task T wants to {read,write} on device D, at offset X, and of size Y". For passthrough IO, it'll also dump the CDB. There's never any actual data traced.
On 4/25/23 12:12 PM, Jens Axboe wrote: > On 4/25/23 11:55?AM, Junxiao Bi wrote: >> Any IO folks can help review this patch? >> >> Paul needs a confirm from you that the information blktrace exporting >> to userspace through the relay files are safe, not leaking information >> that userspace shouldn't know in lockdown mode. > I don't know anything about what lockdown is, but in terms of blktrace, > it is a way to trace meta data associated with IO. It'll tell you things > like "task T wants to {read,write} on device D, at offset X, and of size > Y". For passthrough IO, it'll also dump the CDB. There's never any > actual data traced. Thanks Jens. Lockdown is a security feature which is trying to limit user's(including root) capability to access security sensitive information inside kernel, there are two modes, read-only access is allowed in "integrity" mode while both read and write access are disabled in "confidentiality" mode. Paul, do you have any other concerns regarding blktrace? As Jens mentioned, blktrace just exported IO metadata to Userspace, those were not security sensitive information. Thanks, Junxiao.
On Wed, Apr 26, 2023 at 12:33 PM Junxiao Bi <junxiao.bi@oracle.com> wrote: > Paul, do you have any other concerns regarding blktrace? As Jens > mentioned, blktrace just exported IO metadata to Userspace, those were > not security sensitive information. I think this version of the patchset is much better, thanks for your patience. I don't have any further concerns, and since the lockdown LSM doesn't have a dedicated maintainer I think you should be all set from my perspective. Since there are no changes under security/, I'm assuming this will go in via the tracing tree?
On 4/28/23 2:26 PM, Paul Moore wrote: > On Wed, Apr 26, 2023 at 12:33 PM Junxiao Bi <junxiao.bi@oracle.com> wrote: >> Paul, do you have any other concerns regarding blktrace? As Jens >> mentioned, blktrace just exported IO metadata to Userspace, those were >> not security sensitive information. > I think this version of the patchset is much better, thanks for your > patience. I don't have any further concerns, and since the lockdown > LSM doesn't have a dedicated maintainer I think you should be all set > from my perspective. Thanks a lot for the review. > > Since there are no changes under security/, I'm assuming this will go > in via the tracing tree? Should I notify some specific maintainer or mail list for merging? Thanks, Junxiao. >
On Fri, Apr 28, 2023 at 6:41 PM Junxiao Bi <junxiao.bi@oracle.com> wrote: > On 4/28/23 2:26 PM, Paul Moore wrote: > > On Wed, Apr 26, 2023 at 12:33 PM Junxiao Bi <junxiao.bi@oracle.com> wrote: > >> Paul, do you have any other concerns regarding blktrace? As Jens > >> mentioned, blktrace just exported IO metadata to Userspace, those were > >> not security sensitive information. > > I think this version of the patchset is much better, thanks for your > > patience. I don't have any further concerns, and since the lockdown > > LSM doesn't have a dedicated maintainer I think you should be all set > > from my perspective. > > Thanks a lot for the review. > > > Since there are no changes under security/, I'm assuming this will go > > in via the tracing tree? > > Should I notify some specific maintainer or mail list for merging? When in doubt, the scripts/get_maintainer.pl tool in the kernel sources can be helpful. The results for the debugfs and relay files are fairly generic, but if you look at the results for the blktrace.c file you get a more reasonable list of maintainers and mailing lists. I believe Jens has already commented on your patches at least once, I don't recall if the others have or not. I see you've already CC'd the block mailing list, so that might be enough. Keep in mind that we are in the middle of a merge window so it is very possible this patch might not be merged in a working/next/etc. branch until after the merge window closes (every maintainer is a little bit different in this regard).
On 4/30/23 2:46 PM, Paul Moore wrote: > On Fri, Apr 28, 2023 at 6:41 PM Junxiao Bi <junxiao.bi@oracle.com> wrote: >> On 4/28/23 2:26 PM, Paul Moore wrote: >>> On Wed, Apr 26, 2023 at 12:33 PM Junxiao Bi <junxiao.bi@oracle.com> wrote: >>>> Paul, do you have any other concerns regarding blktrace? As Jens >>>> mentioned, blktrace just exported IO metadata to Userspace, those were >>>> not security sensitive information. >>> I think this version of the patchset is much better, thanks for your >>> patience. I don't have any further concerns, and since the lockdown >>> LSM doesn't have a dedicated maintainer I think you should be all set >>> from my perspective. >> Thanks a lot for the review. >> >>> Since there are no changes under security/, I'm assuming this will go >>> in via the tracing tree? >> Should I notify some specific maintainer or mail list for merging? > When in doubt, the scripts/get_maintainer.pl tool in the kernel > sources can be helpful. > > The results for the debugfs and relay files are fairly generic, but if > you look at the results for the blktrace.c file you get a more > reasonable list of maintainers and mailing lists. I believe Jens has > already commented on your patches at least once, I don't recall if the > others have or not. I see you've already CC'd the block mailing list, > so that might be enough. > > Keep in mind that we are in the middle of a merge window so it is very > possible this patch might not be merged in a working/next/etc. branch > until after the merge window closes (every maintainer is a little bit > different in this regard). I didn't see the patches in the trace tree yet, maybe better to merge it through block tree since it's a blktrace fix. Jens, can you help merge these two patches to your tree? Thanks, Junxiao. >
Hi Paul, The patches have not been merged yet, i would like to resend them, just want to confirm i can add your Reviewed-by in the patches, right? Thanks, Junxiao. On 5/9/23 9:13 AM, Junxiao Bi wrote: > On 4/30/23 2:46 PM, Paul Moore wrote: > >> On Fri, Apr 28, 2023 at 6:41 PM Junxiao Bi <junxiao.bi@oracle.com> >> wrote: >>> On 4/28/23 2:26 PM, Paul Moore wrote: >>>> On Wed, Apr 26, 2023 at 12:33 PM Junxiao Bi <junxiao.bi@oracle.com> >>>> wrote: >>>>> Paul, do you have any other concerns regarding blktrace? As Jens >>>>> mentioned, blktrace just exported IO metadata to Userspace, those >>>>> were >>>>> not security sensitive information. >>>> I think this version of the patchset is much better, thanks for your >>>> patience. I don't have any further concerns, and since the lockdown >>>> LSM doesn't have a dedicated maintainer I think you should be all set >>>> from my perspective. >>> Thanks a lot for the review. >>> >>>> Since there are no changes under security/, I'm assuming this will go >>>> in via the tracing tree? >>> Should I notify some specific maintainer or mail list for merging? >> When in doubt, the scripts/get_maintainer.pl tool in the kernel >> sources can be helpful. >> >> The results for the debugfs and relay files are fairly generic, but if >> you look at the results for the blktrace.c file you get a more >> reasonable list of maintainers and mailing lists. I believe Jens has >> already commented on your patches at least once, I don't recall if the >> others have or not. I see you've already CC'd the block mailing list, >> so that might be enough. >> >> Keep in mind that we are in the middle of a merge window so it is very >> possible this patch might not be merged in a working/next/etc. branch >> until after the merge window closes (every maintainer is a little bit >> different in this regard). > > I didn't see the patches in the trace tree yet, maybe better to merge > it through block tree since it's a blktrace fix. > > Jens, can you help merge these two patches to your tree? > > Thanks, > > Junxiao. > >>
On Fri, May 26, 2023 at 12:56 PM Junxiao Bi <junxiao.bi@oracle.com> wrote: > > Hi Paul, > > The patches have not been merged yet, i would like to resend them, just > want to confirm i can add your Reviewed-by in the patches, right? Hi Junxiao, I haven't personally had the time to verify the blktrace claims that it doesn't violate the Lockdown principles so I'm not comfortable adding my reviewed-by tag at this point in time, I'm sorry. > On 5/9/23 9:13 AM, Junxiao Bi wrote: > > On 4/30/23 2:46 PM, Paul Moore wrote: > > > >> On Fri, Apr 28, 2023 at 6:41 PM Junxiao Bi <junxiao.bi@oracle.com> > >> wrote: > >>> On 4/28/23 2:26 PM, Paul Moore wrote: > >>>> On Wed, Apr 26, 2023 at 12:33 PM Junxiao Bi <junxiao.bi@oracle.com> > >>>> wrote: > >>>>> Paul, do you have any other concerns regarding blktrace? As Jens > >>>>> mentioned, blktrace just exported IO metadata to Userspace, those > >>>>> were > >>>>> not security sensitive information. > >>>> I think this version of the patchset is much better, thanks for your > >>>> patience. I don't have any further concerns, and since the lockdown > >>>> LSM doesn't have a dedicated maintainer I think you should be all set > >>>> from my perspective. > >>> Thanks a lot for the review. > >>> > >>>> Since there are no changes under security/, I'm assuming this will go > >>>> in via the tracing tree? > >>> Should I notify some specific maintainer or mail list for merging? > >> When in doubt, the scripts/get_maintainer.pl tool in the kernel > >> sources can be helpful. > >> > >> The results for the debugfs and relay files are fairly generic, but if > >> you look at the results for the blktrace.c file you get a more > >> reasonable list of maintainers and mailing lists. I believe Jens has > >> already commented on your patches at least once, I don't recall if the > >> others have or not. I see you've already CC'd the block mailing list, > >> so that might be enough. > >> > >> Keep in mind that we are in the middle of a merge window so it is very > >> possible this patch might not be merged in a working/next/etc. branch > >> until after the merge window closes (every maintainer is a little bit > >> different in this regard). > > > > I didn't see the patches in the trace tree yet, maybe better to merge > > it through block tree since it's a blktrace fix. > > > > Jens, can you help merge these two patches to your tree? > > > > Thanks, > > > > Junxiao.
On 5/26/23 2:37 PM, Paul Moore wrote: > On Fri, May 26, 2023 at 12:56 PM Junxiao Bi <junxiao.bi@oracle.com> wrote: >> Hi Paul, >> >> The patches have not been merged yet, i would like to resend them, just >> want to confirm i can add your Reviewed-by in the patches, right? > Hi Junxiao, > > I haven't personally had the time to verify the blktrace claims that > it doesn't violate the Lockdown principles so I'm not comfortable > adding my reviewed-by tag at this point in time, I'm sorry. No problem. With Jens confirmed blktrace only exposed IO metadata to userspace, if any more query regarding blktrace, please let me know. Thank you. Thanks, Junxiao. > >> On 5/9/23 9:13 AM, Junxiao Bi wrote: >>> On 4/30/23 2:46 PM, Paul Moore wrote: >>> >>>> On Fri, Apr 28, 2023 at 6:41 PM Junxiao Bi <junxiao.bi@oracle.com> >>>> wrote: >>>>> On 4/28/23 2:26 PM, Paul Moore wrote: >>>>>> On Wed, Apr 26, 2023 at 12:33 PM Junxiao Bi <junxiao.bi@oracle.com> >>>>>> wrote: >>>>>>> Paul, do you have any other concerns regarding blktrace? As Jens >>>>>>> mentioned, blktrace just exported IO metadata to Userspace, those >>>>>>> were >>>>>>> not security sensitive information. >>>>>> I think this version of the patchset is much better, thanks for your >>>>>> patience. I don't have any further concerns, and since the lockdown >>>>>> LSM doesn't have a dedicated maintainer I think you should be all set >>>>>> from my perspective. >>>>> Thanks a lot for the review. >>>>> >>>>>> Since there are no changes under security/, I'm assuming this will go >>>>>> in via the tracing tree? >>>>> Should I notify some specific maintainer or mail list for merging? >>>> When in doubt, the scripts/get_maintainer.pl tool in the kernel >>>> sources can be helpful. >>>> >>>> The results for the debugfs and relay files are fairly generic, but if >>>> you look at the results for the blktrace.c file you get a more >>>> reasonable list of maintainers and mailing lists. I believe Jens has >>>> already commented on your patches at least once, I don't recall if the >>>> others have or not. I see you've already CC'd the block mailing list, >>>> so that might be enough. >>>> >>>> Keep in mind that we are in the middle of a merge window so it is very >>>> possible this patch might not be merged in a working/next/etc. branch >>>> until after the merge window closes (every maintainer is a little bit >>>> different in this regard). >>> I didn't see the patches in the trace tree yet, maybe better to merge >>> it through block tree since it's a blktrace fix. >>> >>> Jens, can you help merge these two patches to your tree? >>> >>> Thanks, >>> >>> Junxiao.
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c index d5d94510afd3..e1a9f8b7d710 100644 --- a/kernel/trace/blktrace.c +++ b/kernel/trace/blktrace.c @@ -490,10 +490,16 @@ static struct dentry *blk_create_buf_file_callback(const char *filename, &relay_file_operations); } +static bool blk_bypass_lockdown(struct inode *inode) +{ + return true; +} + static const struct rchan_callbacks blk_relay_callbacks = { .subbuf_start = blk_subbuf_start_callback, .create_buf_file = blk_create_buf_file_callback, .remove_buf_file = blk_remove_buf_file_callback, + .bypass_lockdown = blk_bypass_lockdown, }; static void blk_trace_setup_lba(struct blk_trace *bt,
blktrace trace files are per-cpu relay files that are used by kernel to export IO metadata(IO events, type, target disk, offset and len etc.) to userspace, no data from IO itself will be exported. Bypass lockdown for these files will make blktrace work in lockdown mode. Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com> --- kernel/trace/blktrace.c | 6 ++++++ 1 file changed, 6 insertions(+)