Re: [PATCH] Allow softraid crypto to work with write-protected keys
ping
On Wed, 18 May 2016 12:14:50 +0100
bytevolc...@safe-mail.net wrote:
> My apologies for the noise; the previous one was the wrong revision
> (r1.126 instead of 1.127) because both patches look similar; here is
> the most recent catch:
>
> Index: sys/dev/softraid_crypto.c
> ===
> RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
> retrieving revision 1.127
> diff -u -p -r1.127 softraid_crypto.c
> --- sys/dev/softraid_crypto.c 17 May 2016 19:28:59
> - 1.127 +++ sys/dev/softraid_crypto.c 18 May 2016
> 10:26:32 - @@ -795,7 +795,7 @@ sr_crypto_read_key_disk(struct
> sr_discip sr_error(sc, "cannot open key disk %s", devname);
> goto done;
> }
> - if (VOP_OPEN(vn, FREAD | FWRITE, NOCRED, curproc)) {
> + if (VOP_OPEN(vn, FREAD, NOCRED, curproc)) {
> DNPRINTF(SR_D_META,"%s: sr_crypto_read_key_disk
> cannot " "open %s\n", DEVNAME(sc), devname);
> vput(vn);
> @@ -809,8 +809,6 @@ sr_crypto_read_key_disk(struct sr_discip
> NOCRED, curproc)) {
> DNPRINTF(SR_D_META, "%s: sr_crypto_read_key_disk
> ioctl " "failed\n", DEVNAME(sc));
> - VOP_CLOSE(vn, FREAD | FWRITE, NOCRED, curproc);
> - vput(vn);
> goto done;
> }
> if (label.d_partitions[part].p_fstype != FS_RAID) {
>
Re: [PATCH] Allow softraid crypto to work with write-protected keys
My apologies for the noise; the previous one was the wrong revision (r1.126
instead of 1.127) because both patches look similar; here is the most recent
catch:
Index: sys/dev/softraid_crypto.c
===
RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.127
diff -u -p -r1.127 softraid_crypto.c
--- sys/dev/softraid_crypto.c 17 May 2016 19:28:59 - 1.127
+++ sys/dev/softraid_crypto.c 18 May 2016 10:26:32 -
@@ -795,7 +795,7 @@ sr_crypto_read_key_disk(struct sr_discip
sr_error(sc, "cannot open key disk %s", devname);
goto done;
}
- if (VOP_OPEN(vn, FREAD | FWRITE, NOCRED, curproc)) {
+ if (VOP_OPEN(vn, FREAD, NOCRED, curproc)) {
DNPRINTF(SR_D_META,"%s: sr_crypto_read_key_disk cannot "
"open %s\n", DEVNAME(sc), devname);
vput(vn);
@@ -809,8 +809,6 @@ sr_crypto_read_key_disk(struct sr_discip
NOCRED, curproc)) {
DNPRINTF(SR_D_META, "%s: sr_crypto_read_key_disk ioctl "
"failed\n", DEVNAME(sc));
- VOP_CLOSE(vn, FREAD | FWRITE, NOCRED, curproc);
- vput(vn);
goto done;
}
if (label.d_partitions[part].p_fstype != FS_RAID) {
Re: [PATCH] Allow softraid crypto to work with write-protected keys
Ted Unangst wrote:
> i removed these last two lines, since they were incorrect. thanks for spotting
> that. however the vop_close at the end still needs updating.
Thanks Ted,
Also I found another stray VOP_CLOSE() setup.
I have also put in the VOP_OPEN(vn, FREAD, ...) as I am not sure about 100%
clear on what you mean by "the vop_close at the end still needs updating."
Do you mean it needs to actually be called with VOP_CLOSE(vn, FREAD | FWRITE,
...)?
I am also confused by the opening of the key disk in read-write mode, when
there doesn't seem to be any obvious need to write to it; what bit of metadata
needs to be updated? All I see it doing is preventing the possibility of using
write-protected key disks.
Index: sys/dev/softraid_crypto.c
===
RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.126
diff -u -p -r1.126 softraid_crypto.c
--- sys/dev/softraid_crypto.c 12 Apr 2016 16:26:54 - 1.126
+++ sys/dev/softraid_crypto.c 17 May 2016 10:13:02 -
@@ -797,7 +797,7 @@ sr_crypto_read_key_disk(struct sr_discip
sr_error(sc, "cannot open key disk %s", devname);
goto done;
}
- if (VOP_OPEN(vn, FREAD | FWRITE, NOCRED, curproc)) {
+ if (VOP_OPEN(vn, FREAD, NOCRED, curproc)) {
DNPRINTF(SR_D_META,"%s: sr_crypto_read_key_disk cannot "
"open %s\n", DEVNAME(sc), devname);
vput(vn);
@@ -811,8 +811,6 @@ sr_crypto_read_key_disk(struct sr_discip
NOCRED, curproc)) {
DNPRINTF(SR_D_META, "%s: sr_crypto_read_key_disk ioctl "
"failed\n", DEVNAME(sc));
- VOP_CLOSE(vn, FREAD | FWRITE, NOCRED, curproc);
- vput(vn);
goto done;
}
if (label.d_partitions[part].p_fstype != FS_RAID) {
Re: [PATCH] Allow softraid crypto to work with write-protected keys
bytevolc...@safe-mail.net wrote:
>
> Index: sys/dev/softraid_crypto.c
> ===
> RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
> retrieving revision 1.126
> diff -u -p -r1.126 softraid_crypto.c
> --- sys/dev/softraid_crypto.c 12 Apr 2016 16:26:54 -
> 1.126 +++ sys/dev/softraid_crypto.c 17 May 2016 04:18:52 -
> @@ -797,7 +797,7 @@ sr_crypto_read_key_disk(struct sr_discip
> sr_error(sc, "cannot open key disk %s", devname);
> goto done;
> }
> - if (VOP_OPEN(vn, FREAD | FWRITE, NOCRED, curproc)) {
> + if (VOP_OPEN(vn, FREAD, NOCRED, curproc)) {
> DNPRINTF(SR_D_META,"%s: sr_crypto_read_key_disk cannot
> " "open %s\n", DEVNAME(sc), devname);
> vput(vn);
> @@ -811,7 +811,7 @@ sr_crypto_read_key_disk(struct sr_discip
> NOCRED, curproc)) {
> DNPRINTF(SR_D_META, "%s: sr_crypto_read_key_disk ioctl
> " "failed\n", DEVNAME(sc));
> - VOP_CLOSE(vn, FREAD | FWRITE, NOCRED, curproc);
> + VOP_CLOSE(vn, FREAD, NOCRED, curproc);
> vput(vn);
> goto done;
i removed these last two lines, since they were incorrect. thanks for spotting
that. however the vop_close at the end still needs updating.
Re: [PATCH] Allow softraid crypto to work with write-protected keys
I have also noticed that bioctl reports the key disk as
incorrect. I initially thought it was my patch, but it seems to be wrong
whether or not my patch is applied. In fact the contents of the disk do
not change at all:
# bioctl softraid0
Volume Status Size Device
softraid0 0 Online 42197783040 sd3 CRYPTO
0 Online 42197783040 0:0.0 noencl
softraid0 1 Online 33082493440 sd4 CRYPTO
0 Online 33082493440 1:0.0 noencl
1 Online key disk 1:1.0 noencl
softraid0 2 Online 42199204864 sd5 CRYPTO
0 Online 42199204864 2:0.0 noencl
1 Online key disk 2:1.0 noencl
The key disk I use is at sd1 for all volumes mounted (though the chunks
used [sdXa/sdXe/sdXd] are reported correctly).
Tested with the unpatched GENERIC and GENERIC.MP kernel (15/5/16), and
with the write-protect switch off.
I don't really understand why the "sdX" device name is embedded in the
key anyway, since this is likely to change as USB devices are plugged
in, etc. I would greatly appreciate advise/feedback from a dev involved
with softraid on that one.
Improved version of the patch, removes the VOP_CLOSE() that appears to
now be redundant:
Index: sys/dev/softraid_crypto.c
===
RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.126
diff -u -p -r1.126 softraid_crypto.c
--- sys/dev/softraid_crypto.c 12 Apr 2016 16:26:54 -
1.126 +++ sys/dev/softraid_crypto.c 17 May 2016 10:13:02 -
@@ -797,7 +797,7 @@ sr_crypto_read_key_disk(struct sr_discip
sr_error(sc, "cannot open key disk %s", devname);
goto done;
}
- if (VOP_OPEN(vn, FREAD | FWRITE, NOCRED, curproc)) {
+ if (VOP_OPEN(vn, FREAD, NOCRED, curproc)) {
DNPRINTF(SR_D_META,"%s: sr_crypto_read_key_disk cannot
" "open %s\n", DEVNAME(sc), devname);
vput(vn);
@@ -811,8 +811,6 @@ sr_crypto_read_key_disk(struct sr_discip
NOCRED, curproc)) {
DNPRINTF(SR_D_META, "%s: sr_crypto_read_key_disk ioctl
" "failed\n", DEVNAME(sc));
- VOP_CLOSE(vn, FREAD | FWRITE, NOCRED, curproc);
- vput(vn);
goto done;
}
if (label.d_partitions[part].p_fstype != FS_RAID) {
[PATCH] Allow softraid crypto to work with write-protected keys
Softraid currently opens the key disk as read + write. This isn't
necessary when just *reading* from the key disk.
This patch allows for softraid crypto to mount volumes with
write-protected keydisks (eg. Kanguru Flash Blu:
https://www.kanguru.com/storage-accessories/flash-blu30.shtml).
Also tested volume creation with the Flash Blu write-protect switch off;
still works as usual.
Index: sys/dev/softraid_crypto.c
===
RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.126
diff -u -p -r1.126 softraid_crypto.c
--- sys/dev/softraid_crypto.c 12 Apr 2016 16:26:54 -
1.126 +++ sys/dev/softraid_crypto.c 17 May 2016 04:18:52 -
@@ -797,7 +797,7 @@ sr_crypto_read_key_disk(struct sr_discip
sr_error(sc, "cannot open key disk %s", devname);
goto done;
}
- if (VOP_OPEN(vn, FREAD | FWRITE, NOCRED, curproc)) {
+ if (VOP_OPEN(vn, FREAD, NOCRED, curproc)) {
DNPRINTF(SR_D_META,"%s: sr_crypto_read_key_disk cannot
" "open %s\n", DEVNAME(sc), devname);
vput(vn);
@@ -811,7 +811,7 @@ sr_crypto_read_key_disk(struct sr_discip
NOCRED, curproc)) {
DNPRINTF(SR_D_META, "%s: sr_crypto_read_key_disk ioctl
" "failed\n", DEVNAME(sc));
- VOP_CLOSE(vn, FREAD | FWRITE, NOCRED, curproc);
+ VOP_CLOSE(vn, FREAD, NOCRED, curproc);
vput(vn);
goto done;
}
