Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1340298 > unrolled thread
| Started by | Asai Thambi SP <asamymuthupa@micron.com> |
|---|---|
| First post | 2016-02-23 06:00 +0100 |
| Last post | 2016-02-24 03:40 +0100 |
| Articles | 5 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted Asai Thambi SP <asamymuthupa@micron.com> - 2016-02-23 06:00 +0100
Re: [PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted Jens Axboe <axboe@kernel.dk> - 2016-02-23 17:20 +0100
Re: [PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted Asai Thambi SP <asamymuthupa@micron.com> - 2016-02-24 03:10 +0100
Re: [PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted Jens Axboe <axboe@kernel.dk> - 2016-02-24 03:20 +0100
Re: [PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted Asai Thambi SP <asamymuthupa@micron.com> - 2016-02-24 03:40 +0100
| From | Asai Thambi SP <asamymuthupa@micron.com> |
|---|---|
| Date | 2016-02-23 06:00 +0100 |
| Subject | [PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted |
| Message-ID | <r5deO-8N-3@gated-at.bofh.it> |
To avoid erasing a device with a mounted filesystem, try to get exclusive
access to the blkdev object corresponding to the device.
Signed-off-by: Selvan Mani <smani@micron.com>
Signed-off-by: Rajesh Kumar Sambandam <rsambandam@micron.com>
Signed-off-by: Asai Thambi S P <asamymuthupa@micron.com>
---
drivers/block/mtip32xx/mtip32xx.c | 113 ++++++++++++++++++++++++++++++-------
drivers/block/mtip32xx/mtip32xx.h | 2 +
2 files changed, 94 insertions(+), 21 deletions(-)
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 9b180db..37690ab 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -982,6 +982,28 @@ static void mtip_issue_non_ncq_command(struct mtip_port *port, int tag)
port->cmd_issue[MTIP_TAG_INDEX(tag)]);
}
+static inline int mtip_bdev_claim(struct driver_data *dd)
+{
+ if (dd->bdev_claimed == false) {
+ igrab(dd->bdev->bd_inode);
+ if (blkdev_get(dd->bdev, FMODE_EXCL, dd->bdev) < 0) {
+ dev_warn(&dd->pdev->dev, "Drive erase aborted due to non-zero refcount (%d)\n",
+ dd->bdev->bd_holders);
+ return -ERESTARTSYS;
+ }
+ dd->bdev_claimed = true;
+ }
+ return 0;
+}
+
+static inline void mtip_bdev_unclaim(struct driver_data *dd)
+{
+ if (dd->bdev_claimed) {
+ blkdev_put(dd->bdev, FMODE_EXCL);
+ dd->bdev_claimed = false;
+ }
+}
+
static bool mtip_pause_ncq(struct mtip_port *port,
struct host_to_dev_fis *fis)
{
@@ -991,10 +1013,21 @@ static bool mtip_pause_ncq(struct mtip_port *port,
reply = port->rxfis + RX_FIS_D2H_REG;
task_file_data = readl(port->mmio+PORT_TFDATA);
- if ((task_file_data & 1))
+ if ((task_file_data & 1)) {
+ if ((fis->command == ATA_CMD_SEC_SET_PASS) ||
+ (fis->command == ATA_CMD_SEC_ERASE_PREP) ||
+ (fis->command == ATA_CMD_SEC_ERASE_UNIT) ||
+ ((fis->command == 0xFC) &&
+ (fis->features == 0x27 || fis->features == 0x72 ||
+ fis->features == 0x62 || fis->features == 0x26 ||
+ fis->features == 0x12)))
+ mtip_bdev_unclaim(port->dd);
return false;
+ }
- if (fis->command == ATA_CMD_SEC_ERASE_PREP) {
+ if (fis->command == ATA_CMD_SEC_SET_PASS) {
+ mtip_bdev_unclaim(port->dd);
+ } else if (fis->command == ATA_CMD_SEC_ERASE_PREP) {
port->ic_pause_timer = jiffies;
return true;
} else if ((fis->command == ATA_CMD_DOWNLOAD_MICRO) &&
@@ -1005,8 +1038,10 @@ static bool mtip_pause_ncq(struct mtip_port *port,
} else if ((fis->command == ATA_CMD_SEC_ERASE_UNIT) ||
((fis->command == 0xFC) &&
(fis->features == 0x27 || fis->features == 0x72 ||
- fis->features == 0x62 || fis->features == 0x26))) {
+ fis->features == 0x62 || fis->features == 0x26 ||
+ fis->features == 0x12))) {
clear_bit(MTIP_DDF_SEC_LOCK_BIT, &port->dd->dd_flag);
+ mtip_bdev_unclaim(port->dd);
/* Com reset after secure erase or lowlevel format */
mtip_restart_port(port);
clear_bit(MTIP_PF_SE_ACTIVE_BIT, &port->flags);
@@ -1110,8 +1145,23 @@ static int mtip_exec_internal_command(struct mtip_port *port,
set_bit(MTIP_PF_IC_ACTIVE_BIT, &port->flags);
+ /* Check for secure erase while fs mounted */
+ if ((fis->command == ATA_CMD_SEC_SET_PASS) ||
+ (fis->command == ATA_CMD_SEC_ERASE_PREP) ||
+ (fis->command == ATA_CMD_SEC_ERASE_UNIT) ||
+ ((fis->command == 0xFC) &&
+ (fis->features == 0x27 || fis->features == 0x72 ||
+ fis->features == 0x62 || fis->features == 0x26 ||
+ fis->features == 0x12))) {
+ rv = mtip_bdev_claim(dd);
+ if (rv)
+ goto exec_ic_exit;
+ }
+
if (fis->command == ATA_CMD_SEC_ERASE_PREP)
set_bit(MTIP_PF_SE_ACTIVE_BIT, &port->flags);
+ else if (fis->command == ATA_CMD_SEC_ERASE_UNIT)
+ dd->port->ic_pause_timer = 0;
clear_bit(MTIP_PF_DM_ACTIVE_BIT, &port->flags);
@@ -1122,6 +1172,8 @@ static int mtip_exec_internal_command(struct mtip_port *port,
MTIP_QUIESCE_IO_TIMEOUT_MS) < 0) {
dev_warn(&dd->pdev->dev,
"Failed to quiesce IO\n");
+
+ mtip_bdev_unclaim(dd);
mtip_put_int_command(dd, int_cmd);
clear_bit(MTIP_PF_IC_ACTIVE_BIT, &port->flags);
wake_up_interruptible(&port->svc_wait);
@@ -1197,6 +1249,8 @@ static int mtip_exec_internal_command(struct mtip_port *port,
mtip_device_reset(dd); /* recover from timeout issue */
rv = -EAGAIN;
goto exec_ic_exit;
+ } else {
+ rv = 0;
}
} else {
u32 hba_stat, port_stat;
@@ -1247,6 +1301,10 @@ static int mtip_exec_internal_command(struct mtip_port *port,
}
}
exec_ic_exit:
+ if (rv < 0) {
+ clear_bit(MTIP_PF_SE_ACTIVE_BIT, &port->flags);
+ mtip_bdev_unclaim(dd);
+ }
/* Clear the allocated and active bits for the internal command. */
mtip_put_int_command(dd, int_cmd);
clear_bit(MTIP_PF_IC_ACTIVE_BIT, &port->flags);
@@ -1783,6 +1841,7 @@ static int exec_drive_task(struct mtip_port *port, u8 *command)
struct host_to_dev_fis fis;
struct host_to_dev_fis *reply = (port->rxfis + RX_FIS_D2H_REG);
unsigned int to;
+ int rv;
/* Build the FIS. */
memset(&fis, 0, sizeof(struct host_to_dev_fis));
@@ -1809,14 +1868,17 @@ static int exec_drive_task(struct mtip_port *port, u8 *command)
command[6]);
/* Execute the command. */
- if (mtip_exec_internal_command(port,
+ rv = mtip_exec_internal_command(port,
&fis,
5,
0,
0,
0,
GFP_KERNEL,
- to) < 0) {
+ to);
+ if (rv < 0) {
+ if (rv == -ERESTARTSYS)
+ return rv;
return -1;
}
@@ -1905,16 +1967,17 @@ static int exec_drive_command(struct mtip_port *port, u8 *command,
command[3]);
/* Execute the command. */
- if (mtip_exec_internal_command(port,
+ rv = mtip_exec_internal_command(port,
&fis,
5,
(xfer_sz ? dma_addr : 0),
(xfer_sz ? ATA_SECT_SIZE * xfer_sz : 0),
0,
GFP_KERNEL,
- to)
- < 0) {
- rv = -EFAULT;
+ to);
+ if (rv < 0) {
+ if (rv != -ERESTARTSYS)
+ rv = -EFAULT;
goto exit_drive_command;
}
@@ -2152,15 +2215,17 @@ static int exec_drive_taskfile(struct driver_data *dd,
transfer_size = ATA_SECT_SIZE * fis.sect_count;
/* Execute the command.*/
- if (mtip_exec_internal_command(dd->port,
+ err = mtip_exec_internal_command(dd->port,
&fis,
5,
dma_buffer,
transfer_size,
0,
GFP_KERNEL,
- timeout) < 0) {
- err = -EIO;
+ timeout);
+ if (err < 0) {
+ if (err != -ERESTARTSYS)
+ err = -EIO;
goto abort;
}
@@ -2258,6 +2323,8 @@ abort:
static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd,
unsigned long arg)
{
+ int ret = 0;
+
switch (cmd) {
case HDIO_GET_IDENTITY:
{
@@ -2277,10 +2344,11 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd,
return -EFAULT;
/* Execute the drive command. */
- if (exec_drive_command(dd->port,
+ ret = exec_drive_command(dd->port,
drive_command,
- (void __user *) (arg+4)))
- return -EIO;
+ (void __user *) (arg+4));
+ if (ret < 0)
+ return ret;
/* Copy the status back to the users buffer. */
if (copy_to_user((void __user *) arg,
@@ -2301,8 +2369,9 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd,
return -EFAULT;
/* Execute the drive command. */
- if (exec_drive_task(dd->port, drive_command))
- return -EIO;
+ ret = exec_drive_task(dd->port, drive_command);
+ if (ret < 0)
+ return ret;
/* Copy the status back to the users buffer. */
if (copy_to_user((void __user *) arg,
@@ -2314,7 +2383,7 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd,
}
case HDIO_DRIVE_TASKFILE: {
ide_task_request_t req_task;
- int ret, outtotal;
+ int outtotal;
if (copy_from_user(&req_task, (void __user *) arg,
sizeof(req_task)))
@@ -2329,13 +2398,13 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd,
sizeof(req_task)))
return -EFAULT;
- return ret;
+ break;
}
default:
return -EINVAL;
}
- return 0;
+ return ret;
}
/*
@@ -3620,7 +3689,7 @@ static inline bool is_se_active(struct driver_data *dd)
if (time_after(jiffies, to)) {
clear_bit(MTIP_PF_SE_ACTIVE_BIT,
&dd->port->flags);
- clear_bit(MTIP_DDF_SEC_LOCK_BIT, &dd->dd_flag);
+ mtip_bdev_unclaim(dd);
dd->port->ic_pause_timer = 0;
wake_up_interruptible(&dd->port->svc_wait);
return false;
@@ -3934,6 +4003,7 @@ skip_create_disk:
add_disk(dd->disk);
dd->bdev = bdget_disk(dd->disk, 0);
+ dd->bdev_claimed = false;
/*
* Now that the disk is active, initialize any sysfs attributes
* managed by the protocol layer.
@@ -4008,6 +4078,7 @@ static int mtip_block_remove(struct driver_data *dd)
{
struct kobject *kobj;
+ mtip_bdev_unclaim(dd);
mtip_hw_debugfs_exit(dd);
if (dd->mtip_svc_handler) {
diff --git a/drivers/block/mtip32xx/mtip32xx.h b/drivers/block/mtip32xx/mtip32xx.h
index 3274784..29357fd 100644
--- a/drivers/block/mtip32xx/mtip32xx.h
+++ b/drivers/block/mtip32xx/mtip32xx.h
@@ -491,6 +491,8 @@ struct driver_data {
int isr_binding;
+ bool bdev_claimed; /* set when bdev is claimed exclusively */
+
struct block_device *bdev;
struct list_head online_list; /* linkage for online list */
--
1.7.1
[toc] | [next] | [standalone]
| From | Jens Axboe <axboe@kernel.dk> |
|---|---|
| Date | 2016-02-23 17:20 +0100 |
| Message-ID | <r5nQS-7QI-3@gated-at.bofh.it> |
| In reply to | #1340298 |
On Mon, Feb 22 2016, Asai Thambi SP wrote: > > To avoid erasing a device with a mounted filesystem, try to get exclusive > access to the blkdev object corresponding to the device. I don't think this needs to be in the kernel, why not just check from the official format tool if the device is mounted or not? -- Jens Axboe
[toc] | [prev] | [next] | [standalone]
| From | Asai Thambi SP <asamymuthupa@micron.com> |
|---|---|
| Date | 2016-02-24 03:10 +0100 |
| Message-ID | <r5x3Q-656-5@gated-at.bofh.it> |
| In reply to | #1340819 |
On 2/23/2016 8:11 AM, Jens Axboe wrote: > On Mon, Feb 22 2016, Asai Thambi SP wrote: >> >> To avoid erasing a device with a mounted filesystem, try to get exclusive >> access to the blkdev object corresponding to the device. > > I don't think this needs to be in the kernel, why not just check from > the official format tool if the device is mounted or not? > The official format tool checks if the device has a mounted filesystem before starting an erase operation. But with the driver being in kernel, some customers use hdparm to manage the device. This patch prevents possible accidental erase through open source tools.
[toc] | [prev] | [next] | [standalone]
| From | Jens Axboe <axboe@kernel.dk> |
|---|---|
| Date | 2016-02-24 03:20 +0100 |
| Message-ID | <r5xdw-68Q-15@gated-at.bofh.it> |
| In reply to | #1341238 |
On 02/23/2016 07:08 PM, Asai Thambi SP wrote: > On 2/23/2016 8:11 AM, Jens Axboe wrote: >> On Mon, Feb 22 2016, Asai Thambi SP wrote: >>> >>> To avoid erasing a device with a mounted filesystem, try to get exclusive >>> access to the blkdev object corresponding to the device. >> >> I don't think this needs to be in the kernel, why not just check from >> the official format tool if the device is mounted or not? >> > > The official format tool checks if the device has a mounted filesystem before starting an erase operation. But with the driver being in kernel, some customers use hdparm to manage the device. This patch prevents possible accidental erase through open source tools. We generally don't put that kind of policy in the kernel. I can firmware update a drive that is mounted, if I want to shoot myself in the foot, if I want to. The answer is, don't do it... -- Jens Axboe
[toc] | [prev] | [next] | [standalone]
| From | Asai Thambi SP <asamymuthupa@micron.com> |
|---|---|
| Date | 2016-02-24 03:40 +0100 |
| Message-ID | <r5xwR-6i3-13@gated-at.bofh.it> |
| In reply to | #1341246 |
On 2/23/2016 6:14 PM, Jens Axboe wrote: > On 02/23/2016 07:08 PM, Asai Thambi SP wrote: >> On 2/23/2016 8:11 AM, Jens Axboe wrote: >>> On Mon, Feb 22 2016, Asai Thambi SP wrote: >>>> >>>> To avoid erasing a device with a mounted filesystem, try to get exclusive >>>> access to the blkdev object corresponding to the device. >>> >>> I don't think this needs to be in the kernel, why not just check from >>> the official format tool if the device is mounted or not? >>> >> >> The official format tool checks if the device has a mounted filesystem before starting an erase operation. But with the driver being in kernel, some customers use hdparm to manage the device. This patch prevents possible accidental erase through open source tools. > > We generally don't put that kind of policy in the kernel. I can firmware update a drive that is mounted, if I want to shoot myself in the foot, if I want to. The answer is, don't do it... Agreed. I will drop this patch and resend the rest.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web