* [PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted
@ 2016-02-23 4:59 Asai Thambi SP
2016-02-23 16:11 ` Jens Axboe
0 siblings, 1 reply; 5+ messages in thread
From: Asai Thambi SP @ 2016-02-23 4:59 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-kernel, Sam Bradshaw, Selvan Mani, Rajesh Kumar Sambandam,
Vignesh Gunasekaran
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted
2016-02-23 4:59 [PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted Asai Thambi SP
@ 2016-02-23 16:11 ` Jens Axboe
2016-02-24 2:08 ` Asai Thambi SP
0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2016-02-23 16:11 UTC (permalink / raw)
To: Asai Thambi SP
Cc: linux-kernel, Sam Bradshaw, Selvan Mani, Rajesh Kumar Sambandam,
Vignesh Gunasekaran
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted
2016-02-23 16:11 ` Jens Axboe
@ 2016-02-24 2:08 ` Asai Thambi SP
2016-02-24 2:14 ` Jens Axboe
0 siblings, 1 reply; 5+ messages in thread
From: Asai Thambi SP @ 2016-02-24 2:08 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-kernel, Sam Bradshaw, Selvan Mani, Rajesh Kumar Sambandam,
Vignesh Gunasekaran
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted
2016-02-24 2:08 ` Asai Thambi SP
@ 2016-02-24 2:14 ` Jens Axboe
2016-02-24 2:38 ` Asai Thambi SP
0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2016-02-24 2:14 UTC (permalink / raw)
To: Asai Thambi SP
Cc: linux-kernel, Sam Bradshaw, Selvan Mani, Rajesh Kumar Sambandam,
Vignesh Gunasekaran
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted
2016-02-24 2:14 ` Jens Axboe
@ 2016-02-24 2:38 ` Asai Thambi SP
0 siblings, 0 replies; 5+ messages in thread
From: Asai Thambi SP @ 2016-02-24 2:38 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-kernel, Sam Bradshaw, Selvan Mani, Rajesh Kumar Sambandam,
Vignesh Gunasekaran
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-02-24 2:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-23 4:59 [PATCH 01/10] mtip32xx: Abort secure erase when drive is mounted Asai Thambi SP
2016-02-23 16:11 ` Jens Axboe
2016-02-24 2:08 ` Asai Thambi SP
2016-02-24 2:14 ` Jens Axboe
2016-02-24 2:38 ` Asai Thambi SP
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).