| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 03 Sep 2013 16:08:57 -0500
From: Brian King <brking@...ux.vnet.ibm.com>
To: "Philip J. Kelleher" <pjk1939@...ux.vnet.ibm.com>
CC: axboe@...nel.dk, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] rsxx: Handling failed pci_map_page on PowerPC
and double free.
Philip,
I get the following compilation error when applying this patch
and trying to build on x86_32. Once I apply the second patch,
the error goes away, but each patch in the series should be
able to apply and build without requiring future patches.
drivers/block/rsxx/dma.c: In function ‘rsxx_queue_dma’:
drivers/block/rsxx/dma.c:635:28: error: ‘ctrl’ undeclared (first use in this function)
drivers/block/rsxx/dma.c:635:28: note: each undeclared identifier is reported only once for each function it appears in
drivers/block/rsxx/dma.c: In function ‘rsxx_eeh_save_issued_dmas’:
drivers/block/rsxx/dma.c:1055:31: error: ‘ctrl’ undeclared (first use in this function)
drivers/block/rsxx/dma.c: In function ‘rsxx_eeh_remap_dmas’:
drivers/block/rsxx/dma.c:1083:30: error: ‘ctrl’ undeclared (first use in this function)
-Brian
On 08/27/2013 05:58 PM, Philip J. Kelleher wrote:
> From: Philip J Kelleher <pjk1939@...ux.vnet.ibm.com>
>
> The rsxx driver was not checking the correct value during a
> pci_map_page failure. Fixing this also uncovered a
> double free if the bio was returned before it was
> broken up into indiviadual 4k dmas, that is also
> fixed here.
>
> Signed-off-by: Philip J Kelleher <pjk1939@...ux.vnet.ibm.com>
> -------------------------------------------------------------------------------
>
>
> diff -uprN -X linux-block-vanilla/Documentation/dontdiff linux-block-vanilla/drivers/block/rsxx/core.c linux-block/drivers/block/rsxx/core.c
> --- linux-block-vanilla/drivers/block/rsxx/core.c 2013-08-26 15:36:48.915801516 -0500
> +++ linux-block/drivers/block/rsxx/core.c 2013-08-26 15:58:15.572827498 -0500
> @@ -654,7 +654,8 @@ static void rsxx_eeh_failure(struct pci_
> for (i = 0; i < card->n_targets; i++) {
> spin_lock_bh(&card->ctrl[i].queue_lock);
> cnt = rsxx_cleanup_dma_queue(&card->ctrl[i],
> - &card->ctrl[i].queue);
> + &card->ctrl[i].queue,
> + COMPLETE_DMA);
> spin_unlock_bh(&card->ctrl[i].queue_lock);
>
> cnt += rsxx_dma_cancel(&card->ctrl[i]);
> diff -uprN -X linux-block-vanilla/Documentation/dontdiff linux-block-vanilla/drivers/block/rsxx/dma.c linux-block/drivers/block/rsxx/dma.c
> --- linux-block-vanilla/drivers/block/rsxx/dma.c 2013-08-26 15:36:48.966788143 -0500
> +++ linux-block/drivers/block/rsxx/dma.c 2013-08-26 16:00:43.934717159 -0500
> @@ -221,6 +221,19 @@ static void dma_intr_coal_auto_tune(stru
> }
>
> /*----------------- RSXX DMA Handling -------------------*/
> +static void rsxx_free_dma(struct rsxx_dma_ctrl *ctrl, struct rsxx_dma *dma)
> +{
> + if (!pci_dma_mapping_error(ctrl->card->dev, dma->dma_addr)) {
> + pci_unmap_page(ctrl->card->dev, dma->dma_addr,
> + get_dma_size(dma),
> + dma->cmd == HW_CMD_BLK_WRITE ?
> + PCI_DMA_TODEVICE :
> + PCI_DMA_FROMDEVICE);
> + }
> +
> + kmem_cache_free(rsxx_dma_pool, dma);
> +}
> +
> static void rsxx_complete_dma(struct rsxx_dma_ctrl *ctrl,
> struct rsxx_dma *dma,
> unsigned int status)
> @@ -232,21 +245,14 @@ static void rsxx_complete_dma(struct rsx
> if (status & DMA_CANCELLED)
> ctrl->stats.dma_cancelled++;
>
> - if (dma->dma_addr)
> - pci_unmap_page(ctrl->card->dev, dma->dma_addr,
> - get_dma_size(dma),
> - dma->cmd == HW_CMD_BLK_WRITE ?
> - PCI_DMA_TODEVICE :
> - PCI_DMA_FROMDEVICE);
> -
> if (dma->cb)
> dma->cb(ctrl->card, dma->cb_data, status ? 1 : 0);
>
> - kmem_cache_free(rsxx_dma_pool, dma);
> + rsxx_free_dma(ctrl, dma);
> }
>
> int rsxx_cleanup_dma_queue(struct rsxx_dma_ctrl *ctrl,
> - struct list_head *q)
> + struct list_head *q, unsigned int done)
> {
> struct rsxx_dma *dma;
> struct rsxx_dma *tmp;
> @@ -254,7 +260,10 @@ int rsxx_cleanup_dma_queue(struct rsxx_d
>
> list_for_each_entry_safe(dma, tmp, q, list) {
> list_del(&dma->list);
> - rsxx_complete_dma(ctrl, dma, DMA_CANCELLED);
> + if (done & COMPLETE_DMA)
> + rsxx_complete_dma(ctrl, dma, DMA_CANCELLED);
> + else
> + rsxx_free_dma(ctrl, dma);
> cnt++;
> }
>
> @@ -370,7 +379,7 @@ static void dma_engine_stalled(unsigned
>
> /* Clean up the DMA queue */
> spin_lock(&ctrl->queue_lock);
> - cnt = rsxx_cleanup_dma_queue(ctrl, &ctrl->queue);
> + cnt = rsxx_cleanup_dma_queue(ctrl, &ctrl->queue, COMPLETE_DMA);
> spin_unlock(&ctrl->queue_lock);
>
> cnt += rsxx_dma_cancel(ctrl);
> @@ -623,7 +632,7 @@ static int rsxx_queue_dma(struct rsxx_ca
> dma->dma_addr = pci_map_page(card->dev, page, pg_off, dma_len,
> dir ? PCI_DMA_TODEVICE :
> PCI_DMA_FROMDEVICE);
> - if (!dma->dma_addr) {
> + if (pci_dma_mapping_error(ctrl->card->dev, dma->dma_addr)) {
> kmem_cache_free(rsxx_dma_pool, dma);
> return -ENOMEM;
> }
> @@ -736,11 +745,9 @@ int rsxx_dma_queue_bio(struct rsxx_cardi
> return 0;
>
> bvec_err:
> - for (i = 0; i < card->n_targets; i++) {
> - spin_lock_bh(&card->ctrl[i].queue_lock);
> - rsxx_cleanup_dma_queue(&card->ctrl[i], &dma_list[i]);
> - spin_unlock_bh(&card->ctrl[i].queue_lock);
> - }
> + for (i = 0; i < card->n_targets; i++)
> + rsxx_cleanup_dma_queue(&card->ctrl[i], &dma_list[i],
> + FREE_DMA);
>
> return st;
> }
> @@ -990,7 +997,7 @@ void rsxx_dma_destroy(struct rsxx_cardin
>
> /* Clean up the DMA queue */
> spin_lock_bh(&ctrl->queue_lock);
> - rsxx_cleanup_dma_queue(ctrl, &ctrl->queue);
> + rsxx_cleanup_dma_queue(ctrl, &ctrl->queue, COMPLETE_DMA);
> spin_unlock_bh(&ctrl->queue_lock);
>
> rsxx_dma_cancel(ctrl);
> @@ -1045,7 +1052,7 @@ int rsxx_eeh_save_issued_dmas(struct rsx
> card->ctrl[i].e_cnt = 0;
>
> list_for_each_entry(dma, &card->ctrl[i].queue, list) {
> - if (dma->dma_addr)
> + if (!pci_dma_mapping_error(ctrl->card->dev, dma->dma_addr))
> pci_unmap_page(card->dev, dma->dma_addr,
> get_dma_size(dma),
> dma->cmd == HW_CMD_BLK_WRITE ?
> @@ -1073,7 +1080,7 @@ int rsxx_eeh_remap_dmas(struct rsxx_card
> dma->cmd == HW_CMD_BLK_WRITE ?
> PCI_DMA_TODEVICE :
> PCI_DMA_FROMDEVICE);
> - if (!dma->dma_addr) {
> + if (pci_dma_mapping_error(ctrl->card->dev, dma->dma_addr)) {
> spin_unlock_bh(&card->ctrl[i].queue_lock);
> kmem_cache_free(rsxx_dma_pool, dma);
> return -ENOMEM;
> diff -uprN -X linux-block-vanilla/Documentation/dontdiff linux-block-vanilla/drivers/block/rsxx/rsxx_priv.h linux-block/drivers/block/rsxx/rsxx_priv.h
> --- linux-block-vanilla/drivers/block/rsxx/rsxx_priv.h 2013-08-26 15:36:48.996733795 -0500
> +++ linux-block/drivers/block/rsxx/rsxx_priv.h 2013-08-26 15:58:15.576827372 -0500
> @@ -345,6 +345,11 @@ enum rsxx_creg_stat {
> CREG_STAT_TAG_MASK = 0x0000ff00,
> };
>
> +enum rsxx_dma_finish {
> + FREE_DMA = 0x0,
> + COMPLETE_DMA = 0x1,
> +};
> +
> static inline unsigned int CREG_DATA(int N)
> {
> return CREG_DATA0 + (N << 2);
> @@ -379,7 +384,9 @@ typedef void (*rsxx_dma_cb)(struct rsxx_
> int rsxx_dma_setup(struct rsxx_cardinfo *card);
> void rsxx_dma_destroy(struct rsxx_cardinfo *card);
> int rsxx_dma_init(void);
> -int rsxx_cleanup_dma_queue(struct rsxx_dma_ctrl *ctrl, struct list_head *q);
> +int rsxx_cleanup_dma_queue(struct rsxx_dma_ctrl *ctrl,
> + struct list_head *q,
> + unsigned int done);
> int rsxx_dma_cancel(struct rsxx_dma_ctrl *ctrl);
> void rsxx_dma_cleanup(void);
> void rsxx_dma_queue_reset(struct rsxx_cardinfo *card);
>
--
Brian King
Power Linux I/O
IBM Linux Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists