Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-04-05 Thread Scott Bauer
On Thu, Mar 29, 2018 at 08:27:30PM +0200, catch...@ghostav.ddnss.de wrote:
> On Thu, Mar 29, 2018 at 11:16:42AM -0600, Scott Bauer wrote:
> > Yeah, having to autheticate to write the MBR is a real bummer. Theoretically
> > you could dd a the pw struct + the shador MBR into sysfs. But that's
> > a pretty disgusting hack just to use sysfs. The other method I thought of
> > was to authenticate via ioctl then write via sysfs. We already save the PW
> > in-kernel for unlocks, so perhaps we can re-use the save-for-unlock to
> > do shadow MBR writes via sysfs?
> > 
> > Re-using an already exposed ioctl for another purpose seems somewhat 
> > dangerous?
> > In the sense that what if the user wants to write the smbr but doesn't want 
> > to
> > unlock on suspends, or does not want their PW hanging around in the kernel.
> Well. If we would force the user to a two-step interaction, why not stay
> completely in sysfs? So instead of using the save-for-unlock ioctl, we
> could export each security provider( (AdminSP, UserSPX, ...) as a sysfs

The Problem with this is Single user mode, where you can assign users to 
locking ranges.
There would have to be a lot of dynamic changes of sysfs as users get 
added/removed,
or added to LRs etc. It seems like we're trying mold something that already
works fine into something that doesnt really work as we dig into the details. 



> directory with appropriate files (e.g. mbr for AdminSP) as well as a
> 'unlock' file to store a users password for the specific locking space
> and a 'lock' file to remove the stored password on write to it.
> Of course, while this will prevent from reuse of the ioctl and
> stays within the same configuration method, the PW will still hang
> around in the kernel between 'lock' and 'unlock'.
> 
> Another idea I just came across while writing this down:
> Instead of storing/releasing the password permanently with the 'unlock' and
> 'lock' files, those may be used to start/stop an authenticated session.
> To make it more clear what I mean: Each ioctl that requires
> authentication has a similar pattern:
> discovery0, start_session, , end_session
> Instead of having the combination determined by the ioctl, the 'unlock'
> would do discovery0 and start_session while the 'lock' would do the
> end_session. The user is free to issue further commands with the
> appropriate write/reads to other files of the sysfs-directory.
> While this removes the requirement to store the key within kernel space,
> the open session handle may be used from everybody with permissions for
> read/write access to the sysfs-directory files. So this is not optimal
> as not only the user who provided the password will finally be able to use
> it.

I generally like the idea of being able to run your abritrary opal commands, 
but:

that's probably not going to work for the final reason you outlined.
Even though it's root only access(to sysfs) we're breaking the authentication
lower down by essentially allowing any opal command to be ran if you've somehow
become root.

The other issue with this is the session time out in opal. When we dispatch the 
commands
in-kernel we're hammering them out 1-by-1. If the user needs to do an 
activatelsp,
setuplr, etc. They do that with a new session.

If someone starts the session and it times out it may be hard to figure out how 
to not
get an SP_BUSY back from the controller. I've in the past just had to wipe my 
damn
fw to get out of SP_BUSYs, but that could be due to the early implementations I 
was
dealing with.



> I already did some basic work to split of the session-information from
> the opal_dev struct (initially to reduce the memory-footprint of devices with
> currently no active opal-interaction). So I think, I could get a
> proof-of-concept of this approach within the next one or two weeks if
> there are no objections to the base idea.

Sorry to ocme back a week later, but if you do have anything it would be at 
least
interesting to see. I would still prefer the ioctl route, but will review and 
test
any implementation people deem acceptable. 


Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-04-05 Thread Scott Bauer
On Thu, Mar 29, 2018 at 08:27:30PM +0200, catch...@ghostav.ddnss.de wrote:
> On Thu, Mar 29, 2018 at 11:16:42AM -0600, Scott Bauer wrote:
> > Yeah, having to autheticate to write the MBR is a real bummer. Theoretically
> > you could dd a the pw struct + the shador MBR into sysfs. But that's
> > a pretty disgusting hack just to use sysfs. The other method I thought of
> > was to authenticate via ioctl then write via sysfs. We already save the PW
> > in-kernel for unlocks, so perhaps we can re-use the save-for-unlock to
> > do shadow MBR writes via sysfs?
> > 
> > Re-using an already exposed ioctl for another purpose seems somewhat 
> > dangerous?
> > In the sense that what if the user wants to write the smbr but doesn't want 
> > to
> > unlock on suspends, or does not want their PW hanging around in the kernel.
> Well. If we would force the user to a two-step interaction, why not stay
> completely in sysfs? So instead of using the save-for-unlock ioctl, we
> could export each security provider( (AdminSP, UserSPX, ...) as a sysfs

The Problem with this is Single user mode, where you can assign users to 
locking ranges.
There would have to be a lot of dynamic changes of sysfs as users get 
added/removed,
or added to LRs etc. It seems like we're trying mold something that already
works fine into something that doesnt really work as we dig into the details. 



> directory with appropriate files (e.g. mbr for AdminSP) as well as a
> 'unlock' file to store a users password for the specific locking space
> and a 'lock' file to remove the stored password on write to it.
> Of course, while this will prevent from reuse of the ioctl and
> stays within the same configuration method, the PW will still hang
> around in the kernel between 'lock' and 'unlock'.
> 
> Another idea I just came across while writing this down:
> Instead of storing/releasing the password permanently with the 'unlock' and
> 'lock' files, those may be used to start/stop an authenticated session.
> To make it more clear what I mean: Each ioctl that requires
> authentication has a similar pattern:
> discovery0, start_session, , end_session
> Instead of having the combination determined by the ioctl, the 'unlock'
> would do discovery0 and start_session while the 'lock' would do the
> end_session. The user is free to issue further commands with the
> appropriate write/reads to other files of the sysfs-directory.
> While this removes the requirement to store the key within kernel space,
> the open session handle may be used from everybody with permissions for
> read/write access to the sysfs-directory files. So this is not optimal
> as not only the user who provided the password will finally be able to use
> it.

I generally like the idea of being able to run your abritrary opal commands, 
but:

that's probably not going to work for the final reason you outlined.
Even though it's root only access(to sysfs) we're breaking the authentication
lower down by essentially allowing any opal command to be ran if you've somehow
become root.

The other issue with this is the session time out in opal. When we dispatch the 
commands
in-kernel we're hammering them out 1-by-1. If the user needs to do an 
activatelsp,
setuplr, etc. They do that with a new session.

If someone starts the session and it times out it may be hard to figure out how 
to not
get an SP_BUSY back from the controller. I've in the past just had to wipe my 
damn
fw to get out of SP_BUSYs, but that could be due to the early implementations I 
was
dealing with.



> I already did some basic work to split of the session-information from
> the opal_dev struct (initially to reduce the memory-footprint of devices with
> currently no active opal-interaction). So I think, I could get a
> proof-of-concept of this approach within the next one or two weeks if
> there are no objections to the base idea.

Sorry to ocme back a week later, but if you do have anything it would be at 
least
interesting to see. I would still prefer the ioctl route, but will review and 
test
any implementation people deem acceptable. 


Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-29 Thread catchall
On Thu, Mar 29, 2018 at 11:16:42AM -0600, Scott Bauer wrote:
> Yeah, having to autheticate to write the MBR is a real bummer. Theoretically
> you could dd a the pw struct + the shador MBR into sysfs. But that's
> a pretty disgusting hack just to use sysfs. The other method I thought of
> was to authenticate via ioctl then write via sysfs. We already save the PW
> in-kernel for unlocks, so perhaps we can re-use the save-for-unlock to
> do shadow MBR writes via sysfs?
> 
> Re-using an already exposed ioctl for another purpose seems somewhat 
> dangerous?
> In the sense that what if the user wants to write the smbr but doesn't want to
> unlock on suspends, or does not want their PW hanging around in the kernel.
Well. If we would force the user to a two-step interaction, why not stay
completely in sysfs? So instead of using the save-for-unlock ioctl, we
could export each security provider( (AdminSP, UserSPX, ...) as a sysfs
directory with appropriate files (e.g. mbr for AdminSP) as well as a
'unlock' file to store a users password for the specific locking space
and a 'lock' file to remove the stored password on write to it.
Of course, while this will prevent from reuse of the ioctl and
stays within the same configuration method, the PW will still hang
around in the kernel between 'lock' and 'unlock'.

Another idea I just came across while writing this down:
Instead of storing/releasing the password permanently with the 'unlock' and
'lock' files, those may be used to start/stop an authenticated session.
To make it more clear what I mean: Each ioctl that requires
authentication has a similar pattern:
discovery0, start_session, , end_session
Instead of having the combination determined by the ioctl, the 'unlock'
would do discovery0 and start_session while the 'lock' would do the
end_session. The user is free to issue further commands with the
appropriate write/reads to other files of the sysfs-directory.
While this removes the requirement to store the key within kernel space,
the open session handle may be used from everybody with permissions for
read/write access to the sysfs-directory files. So this is not optimal
as not only the user who provided the password will finally be able to use
it.
I already did some basic work to split of the session-information from
the opal_dev struct (initially to reduce the memory-footprint of devices with
currently no active opal-interaction). So I think, I could get a
proof-of-concept of this approach within the next one or two weeks if
there are no objections to the base idea.

Thank you,
 Jonas


Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-29 Thread catchall
On Thu, Mar 29, 2018 at 11:16:42AM -0600, Scott Bauer wrote:
> Yeah, having to autheticate to write the MBR is a real bummer. Theoretically
> you could dd a the pw struct + the shador MBR into sysfs. But that's
> a pretty disgusting hack just to use sysfs. The other method I thought of
> was to authenticate via ioctl then write via sysfs. We already save the PW
> in-kernel for unlocks, so perhaps we can re-use the save-for-unlock to
> do shadow MBR writes via sysfs?
> 
> Re-using an already exposed ioctl for another purpose seems somewhat 
> dangerous?
> In the sense that what if the user wants to write the smbr but doesn't want to
> unlock on suspends, or does not want their PW hanging around in the kernel.
Well. If we would force the user to a two-step interaction, why not stay
completely in sysfs? So instead of using the save-for-unlock ioctl, we
could export each security provider( (AdminSP, UserSPX, ...) as a sysfs
directory with appropriate files (e.g. mbr for AdminSP) as well as a
'unlock' file to store a users password for the specific locking space
and a 'lock' file to remove the stored password on write to it.
Of course, while this will prevent from reuse of the ioctl and
stays within the same configuration method, the PW will still hang
around in the kernel between 'lock' and 'unlock'.

Another idea I just came across while writing this down:
Instead of storing/releasing the password permanently with the 'unlock' and
'lock' files, those may be used to start/stop an authenticated session.
To make it more clear what I mean: Each ioctl that requires
authentication has a similar pattern:
discovery0, start_session, , end_session
Instead of having the combination determined by the ioctl, the 'unlock'
would do discovery0 and start_session while the 'lock' would do the
end_session. The user is free to issue further commands with the
appropriate write/reads to other files of the sysfs-directory.
While this removes the requirement to store the key within kernel space,
the open session handle may be used from everybody with permissions for
read/write access to the sysfs-directory files. So this is not optimal
as not only the user who provided the password will finally be able to use
it.
I already did some basic work to split of the session-information from
the opal_dev struct (initially to reduce the memory-footprint of devices with
currently no active opal-interaction). So I think, I could get a
proof-of-concept of this approach within the next one or two weeks if
there are no objections to the base idea.

Thank you,
 Jonas


Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-29 Thread Scott Bauer
On Thu, Mar 29, 2018 at 07:30:02PM +0200, Jonas Rabenstein wrote:
> Hi,
> On Wed, Mar 21, 2018 at 02:43:21AM +0100, Jonas Rabenstein wrote:
> > On Tue, Mar 20, 2018 at 04:09:08PM -0600, Scott Bauer wrote:
> > > On Tue, Mar 20, 2018 at 10:36:04AM +0100, Jonas Rabenstein wrote:
> > > > On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote:
> > > > > On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote:
> > > > > I hate doing this as an ioctls.  Can we make this a sysfs binary file
> > > > > so that people can use dd or cat to write the shadow mbr?
> > > > I already thought about providing a sysfs interface for all that instead
> > > > of using ioctls. But as I am pretty new to kernel programming I do not
> > > > have all the required insight. Especially, as writing the mbr requires
> > > > the sed-opal password I am unsure how a clean sysfs interface to provide
> > > > the password together with a simple dd would look like.
> Just wanted to ask, how to proceed with those patches/what I should do.
> Using sysfs instead of an ioctl is probably easier to use from userspace
> _if_ there is a good way to provide the password - which I do not know
> of :(
> If nobody else could think of a solution, shall writes to the shadow mbr
> remain unsupported?
> 
> I'ld really appreciate feedback and possible solutions,
>  Jonas

Yeah, having to autheticate to write the MBR is a real bummer. Theoretically
you could dd a the pw struct + the shador MBR into sysfs. But that's
a pretty disgusting hack just to use sysfs. The other method I thought of
was to authenticate via ioctl then write via sysfs. We already save the PW
in-kernel for unlocks, so perhaps we can re-use the save-for-unlock to
do shadow MBR writes via sysfs?

Re-using an already exposed ioctl for another purpose seems somewhat dangerous?
In the sense that what if the user wants to write the smbr but doesn't want to
unlock on suspends, or does not want their PW hanging around in the kernel.

Overall I think the ioctl is still the best path forward due to the 
authentication
problem. But am still willing to hear others opinions if they do have an idea.

I can say yes to the Ioctl, but we really need Jens and Christophs Okay on it.

I have some free time tomorrow to work on this, so let me goof with it tomorrow 
and
over the weekend and I'll see if there is a sane way to get sysfs to work.



Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-29 Thread Scott Bauer
On Thu, Mar 29, 2018 at 07:30:02PM +0200, Jonas Rabenstein wrote:
> Hi,
> On Wed, Mar 21, 2018 at 02:43:21AM +0100, Jonas Rabenstein wrote:
> > On Tue, Mar 20, 2018 at 04:09:08PM -0600, Scott Bauer wrote:
> > > On Tue, Mar 20, 2018 at 10:36:04AM +0100, Jonas Rabenstein wrote:
> > > > On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote:
> > > > > On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote:
> > > > > I hate doing this as an ioctls.  Can we make this a sysfs binary file
> > > > > so that people can use dd or cat to write the shadow mbr?
> > > > I already thought about providing a sysfs interface for all that instead
> > > > of using ioctls. But as I am pretty new to kernel programming I do not
> > > > have all the required insight. Especially, as writing the mbr requires
> > > > the sed-opal password I am unsure how a clean sysfs interface to provide
> > > > the password together with a simple dd would look like.
> Just wanted to ask, how to proceed with those patches/what I should do.
> Using sysfs instead of an ioctl is probably easier to use from userspace
> _if_ there is a good way to provide the password - which I do not know
> of :(
> If nobody else could think of a solution, shall writes to the shadow mbr
> remain unsupported?
> 
> I'ld really appreciate feedback and possible solutions,
>  Jonas

Yeah, having to autheticate to write the MBR is a real bummer. Theoretically
you could dd a the pw struct + the shador MBR into sysfs. But that's
a pretty disgusting hack just to use sysfs. The other method I thought of
was to authenticate via ioctl then write via sysfs. We already save the PW
in-kernel for unlocks, so perhaps we can re-use the save-for-unlock to
do shadow MBR writes via sysfs?

Re-using an already exposed ioctl for another purpose seems somewhat dangerous?
In the sense that what if the user wants to write the smbr but doesn't want to
unlock on suspends, or does not want their PW hanging around in the kernel.

Overall I think the ioctl is still the best path forward due to the 
authentication
problem. But am still willing to hear others opinions if they do have an idea.

I can say yes to the Ioctl, but we really need Jens and Christophs Okay on it.

I have some free time tomorrow to work on this, so let me goof with it tomorrow 
and
over the weekend and I'll see if there is a sane way to get sysfs to work.



Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-29 Thread Jonas Rabenstein
Hi,
On Wed, Mar 21, 2018 at 02:43:21AM +0100, Jonas Rabenstein wrote:
> On Tue, Mar 20, 2018 at 04:09:08PM -0600, Scott Bauer wrote:
> > On Tue, Mar 20, 2018 at 10:36:04AM +0100, Jonas Rabenstein wrote:
> > > On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote:
> > > > On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote:
> > > > I hate doing this as an ioctls.  Can we make this a sysfs binary file
> > > > so that people can use dd or cat to write the shadow mbr?
> > > I already thought about providing a sysfs interface for all that instead
> > > of using ioctls. But as I am pretty new to kernel programming I do not
> > > have all the required insight. Especially, as writing the mbr requires
> > > the sed-opal password I am unsure how a clean sysfs interface to provide
> > > the password together with a simple dd would look like.
Just wanted to ask, how to proceed with those patches/what I should do.
Using sysfs instead of an ioctl is probably easier to use from userspace
_if_ there is a good way to provide the password - which I do not know
of :(
If nobody else could think of a solution, shall writes to the shadow mbr
remain unsupported?

I'ld really appreciate feedback and possible solutions,
 Jonas


Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-29 Thread Jonas Rabenstein
Hi,
On Wed, Mar 21, 2018 at 02:43:21AM +0100, Jonas Rabenstein wrote:
> On Tue, Mar 20, 2018 at 04:09:08PM -0600, Scott Bauer wrote:
> > On Tue, Mar 20, 2018 at 10:36:04AM +0100, Jonas Rabenstein wrote:
> > > On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote:
> > > > On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote:
> > > > I hate doing this as an ioctls.  Can we make this a sysfs binary file
> > > > so that people can use dd or cat to write the shadow mbr?
> > > I already thought about providing a sysfs interface for all that instead
> > > of using ioctls. But as I am pretty new to kernel programming I do not
> > > have all the required insight. Especially, as writing the mbr requires
> > > the sed-opal password I am unsure how a clean sysfs interface to provide
> > > the password together with a simple dd would look like.
Just wanted to ask, how to proceed with those patches/what I should do.
Using sysfs instead of an ioctl is probably easier to use from userspace
_if_ there is a good way to provide the password - which I do not know
of :(
If nobody else could think of a solution, shall writes to the shadow mbr
remain unsupported?

I'ld really appreciate feedback and possible solutions,
 Jonas


Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-20 Thread Jonas Rabenstein
On Tue, Mar 20, 2018 at 04:09:08PM -0600, Scott Bauer wrote:
> On Tue, Mar 20, 2018 at 10:36:04AM +0100, Jonas Rabenstein wrote:
> > On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote:
> > > On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote:
> > > > Allow modification of the shadow mbr. If the shadow mbr is not marked as
> > > > done, this data will be presented read only as the device content. Only
> > > > after marking the shadow mbr as done and unlocking a locking range the
> > > > actual content is accessible.
> > > 
> > > I hate doing this as an ioctls.  Can we make this a sysfs binary file
> > > so that people can use dd or cat to write the shadow mbr?
> > I already thought about providing a sysfs interface for all that instead
> > of using ioctls. But as I am pretty new to kernel programming I do not
> > have all the required insight. Especially, as writing the mbr requires
> > the sed-opal password I am unsure how a clean sysfs interface to provide
> > the password together with a simple dd would look like.
> > Moreover I already have a patch that changes the 'void *data' argument
> > to setup_opal_dev to a kobject pointer. As far as I know, this is the
> > first step to get into the sysfs hierarchy. But as I do not have access
> > to an NVMe drive and have no idea about its implementation, this change
> > works only for the scsi side.
> 
> Post what you have as an RFC (review for comment) and I will test for the NVMe
> side, and or start a port for NVMe. It doesn't have to be perfect since you're
> sending it out as RFC. It's just a base for us to test/look at to see if we
> still like the sysfs way.
Seems, like I failed to make my point in the previous message. I do not
have more than adding a directory 'sed_opal' in sysfs for each sed-opal
enabled disk. But that directory is completely empty. My further plans
are, to fill up that directory with some public info like the one that
gets printed by a call to TCG's 'sedutil-cli --query' command.
The interesting part - where I am clueless how to achieve it -  would be
to have a binary sysfs attribute (like mentioned by Christoph) for the
features (like shadow mbr) requiring some kind of authentication. Until
there is any (good) idea, I do not think it is time for an RFC?

-- Jonas


Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-20 Thread Jonas Rabenstein
On Tue, Mar 20, 2018 at 04:09:08PM -0600, Scott Bauer wrote:
> On Tue, Mar 20, 2018 at 10:36:04AM +0100, Jonas Rabenstein wrote:
> > On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote:
> > > On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote:
> > > > Allow modification of the shadow mbr. If the shadow mbr is not marked as
> > > > done, this data will be presented read only as the device content. Only
> > > > after marking the shadow mbr as done and unlocking a locking range the
> > > > actual content is accessible.
> > > 
> > > I hate doing this as an ioctls.  Can we make this a sysfs binary file
> > > so that people can use dd or cat to write the shadow mbr?
> > I already thought about providing a sysfs interface for all that instead
> > of using ioctls. But as I am pretty new to kernel programming I do not
> > have all the required insight. Especially, as writing the mbr requires
> > the sed-opal password I am unsure how a clean sysfs interface to provide
> > the password together with a simple dd would look like.
> > Moreover I already have a patch that changes the 'void *data' argument
> > to setup_opal_dev to a kobject pointer. As far as I know, this is the
> > first step to get into the sysfs hierarchy. But as I do not have access
> > to an NVMe drive and have no idea about its implementation, this change
> > works only for the scsi side.
> 
> Post what you have as an RFC (review for comment) and I will test for the NVMe
> side, and or start a port for NVMe. It doesn't have to be perfect since you're
> sending it out as RFC. It's just a base for us to test/look at to see if we
> still like the sysfs way.
Seems, like I failed to make my point in the previous message. I do not
have more than adding a directory 'sed_opal' in sysfs for each sed-opal
enabled disk. But that directory is completely empty. My further plans
are, to fill up that directory with some public info like the one that
gets printed by a call to TCG's 'sedutil-cli --query' command.
The interesting part - where I am clueless how to achieve it -  would be
to have a binary sysfs attribute (like mentioned by Christoph) for the
features (like shadow mbr) requiring some kind of authentication. Until
there is any (good) idea, I do not think it is time for an RFC?

-- Jonas


Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-20 Thread Scott Bauer
On Tue, Mar 20, 2018 at 10:36:04AM +0100, Jonas Rabenstein wrote:
> On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote:
> > On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote:
> > > Allow modification of the shadow mbr. If the shadow mbr is not marked as
> > > done, this data will be presented read only as the device content. Only
> > > after marking the shadow mbr as done and unlocking a locking range the
> > > actual content is accessible.
> > 
> > I hate doing this as an ioctls.  Can we make this a sysfs binary file
> > so that people can use dd or cat to write the shadow mbr?
> I already thought about providing a sysfs interface for all that instead
> of using ioctls. But as I am pretty new to kernel programming I do not
> have all the required insight. Especially, as writing the mbr requires
> the sed-opal password I am unsure how a clean sysfs interface to provide
> the password together with a simple dd would look like.
> Moreover I already have a patch that changes the 'void *data' argument
> to setup_opal_dev to a kobject pointer. As far as I know, this is the
> first step to get into the sysfs hierarchy. But as I do not have access
> to an NVMe drive and have no idea about its implementation, this change
> works only for the scsi side.

Post what you have as an RFC (review for comment) and I will test for the NVMe
side, and or start a port for NVMe. It doesn't have to be perfect since you're
sending it out as RFC. It's just a base for us to test/look at to see if we
still like the sysfs way.





Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-20 Thread Scott Bauer
On Tue, Mar 20, 2018 at 10:36:04AM +0100, Jonas Rabenstein wrote:
> On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote:
> > On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote:
> > > Allow modification of the shadow mbr. If the shadow mbr is not marked as
> > > done, this data will be presented read only as the device content. Only
> > > after marking the shadow mbr as done and unlocking a locking range the
> > > actual content is accessible.
> > 
> > I hate doing this as an ioctls.  Can we make this a sysfs binary file
> > so that people can use dd or cat to write the shadow mbr?
> I already thought about providing a sysfs interface for all that instead
> of using ioctls. But as I am pretty new to kernel programming I do not
> have all the required insight. Especially, as writing the mbr requires
> the sed-opal password I am unsure how a clean sysfs interface to provide
> the password together with a simple dd would look like.
> Moreover I already have a patch that changes the 'void *data' argument
> to setup_opal_dev to a kobject pointer. As far as I know, this is the
> first step to get into the sysfs hierarchy. But as I do not have access
> to an NVMe drive and have no idea about its implementation, this change
> works only for the scsi side.

Post what you have as an RFC (review for comment) and I will test for the NVMe
side, and or start a port for NVMe. It doesn't have to be perfect since you're
sending it out as RFC. It's just a base for us to test/look at to see if we
still like the sysfs way.





Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-20 Thread Jonas Rabenstein
On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote:
> On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote:
> > Allow modification of the shadow mbr. If the shadow mbr is not marked as
> > done, this data will be presented read only as the device content. Only
> > after marking the shadow mbr as done and unlocking a locking range the
> > actual content is accessible.
> 
> I hate doing this as an ioctls.  Can we make this a sysfs binary file
> so that people can use dd or cat to write the shadow mbr?
I already thought about providing a sysfs interface for all that instead
of using ioctls. But as I am pretty new to kernel programming I do not
have all the required insight. Especially, as writing the mbr requires
the sed-opal password I am unsure how a clean sysfs interface to provide
the password together with a simple dd would look like.
Moreover I already have a patch that changes the 'void *data' argument
to setup_opal_dev to a kobject pointer. As far as I know, this is the
first step to get into the sysfs hierarchy. But as I do not have access
to an NVMe drive and have no idea about its implementation, this change
works only for the scsi side.
In other words, if someone could hint me in the right direction, I would
be glad to (re)implement the ioctl interface for sysfs. Moreover, this
would allow to export some additional information like the current state
of the device (is it looke, is sed-opal enabled, whats the current state
of mbr, etc.).

- Jonas


Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-20 Thread Jonas Rabenstein
On Mon, Mar 19, 2018 at 08:52:24PM +0100, Christoph Hellwig wrote:
> On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote:
> > Allow modification of the shadow mbr. If the shadow mbr is not marked as
> > done, this data will be presented read only as the device content. Only
> > after marking the shadow mbr as done and unlocking a locking range the
> > actual content is accessible.
> 
> I hate doing this as an ioctls.  Can we make this a sysfs binary file
> so that people can use dd or cat to write the shadow mbr?
I already thought about providing a sysfs interface for all that instead
of using ioctls. But as I am pretty new to kernel programming I do not
have all the required insight. Especially, as writing the mbr requires
the sed-opal password I am unsure how a clean sysfs interface to provide
the password together with a simple dd would look like.
Moreover I already have a patch that changes the 'void *data' argument
to setup_opal_dev to a kobject pointer. As far as I know, this is the
first step to get into the sysfs hierarchy. But as I do not have access
to an NVMe drive and have no idea about its implementation, this change
works only for the scsi side.
In other words, if someone could hint me in the right direction, I would
be glad to (re)implement the ioctl interface for sysfs. Moreover, this
would allow to export some additional information like the current state
of the device (is it looke, is sed-opal enabled, whats the current state
of mbr, etc.).

- Jonas


Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-19 Thread Christoph Hellwig
On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote:
> Allow modification of the shadow mbr. If the shadow mbr is not marked as
> done, this data will be presented read only as the device content. Only
> after marking the shadow mbr as done and unlocking a locking range the
> actual content is accessible.

I hate doing this as an ioctls.  Can we make this a sysfs binary file
so that people can use dd or cat to write the shadow mbr?


Re: [PATCH v2 08/11] block: sed-opal: ioctl for writing to shadow mbr

2018-03-19 Thread Christoph Hellwig
On Mon, Mar 19, 2018 at 07:36:50PM +0100, Jonas Rabenstein wrote:
> Allow modification of the shadow mbr. If the shadow mbr is not marked as
> done, this data will be presented read only as the device content. Only
> after marking the shadow mbr as done and unlocking a locking range the
> actual content is accessible.

I hate doing this as an ioctls.  Can we make this a sysfs binary file
so that people can use dd or cat to write the shadow mbr?