| 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
| ||
|
Message-Id: <200903011450.04274.eike-kernel@sf-tec.de>
Date: Sun, 1 Mar 2009 14:49:52 +0100
From: Rolf Eike Beer <eike-kernel@...tec.de>
To: Mike Miller <mike.miller@...com>
Cc: jens.axboe@...cle.com, fujita.tomonori@....ntt.co.jp,
Andrew Morton <akpm@...ux-foundation.org>,
LKML <linux-kernel@...r.kernel.org>,
"LKML-scsi" <linux-scsi@...r.kernel.org>, coldwell@...hat.com,
hare@...ell.com, iss_storagedev@...com, iss.sbteam@...com
Subject: Re: [PATCH] hpsa: SCSI driver for HP Smart Array controllers
You wrote:
> +static int adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
> + struct hpsa_scsi_dev_t sd[], int nsds)
> +{
> + /* sd contains scsi3 addresses and devtypes, and inquiry */
> + /* data. This function takes what's in sd to be the current */
> + /* reality and updates h->dev[] to reflect that reality. */
> +
> + int i, entry, device_change, changes = 0;
> + struct hpsa_scsi_dev_t *csd;
> + unsigned long flags;
> + struct scsi2map *added, *removed;
> + int nadded, nremoved;
> + struct Scsi_Host *sh = NULL;
> +
> + added = kzalloc(sizeof(*added) * HPSA_MAX_SCSI_DEVS_PER_HBA,
> + GFP_KERNEL);
> + removed = kzalloc(sizeof(*removed) * HPSA_MAX_SCSI_DEVS_PER_HBA,
> + GFP_KERNEL);
kcalloc()?
> +static struct CommandList_struct *cmd_alloc(struct ctlr_info *h)
> +{
> + struct CommandList_struct *c;
> + int i;
> + union u64bit temp64;
> + dma_addr_t cmd_dma_handle, err_dma_handle;
> +
> + do {
> + i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
> + if (i == h->nr_cmds)
> + return NULL;
> + } while (test_and_set_bit
> + (i & (BITS_PER_LONG - 1),
> + h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0);
> + c = h->cmd_pool + i;
> + memset(c, 0, sizeof(struct CommandList_struct));
sizeof(*c)
> + cmd_dma_handle = h->cmd_pool_dhandle
> + + i * sizeof(struct CommandList_struct);
> + c->err_info = h->errinfo_pool + i;
> + memset(c->err_info, 0, sizeof(struct ErrorInfo_struct));
sizeof(c->err_info)
> + err_dma_handle = h->errinfo_pool_dhandle
> + + i * sizeof(struct ErrorInfo_struct);
> + h->nr_allocs++;
> +
> + c->cmdindex = i;
> +
> + INIT_HLIST_NODE(&c->list);
> + c->busaddr = (__u32) cmd_dma_handle;
> + temp64.val = (__u64) err_dma_handle;
> + c->ErrDesc.Addr.lower = temp64.val32.lower;
> + c->ErrDesc.Addr.upper = temp64.val32.upper;
> + c->ErrDesc.Len = sizeof(struct ErrorInfo_struct);
> +
> + c->ctlr = h->ctlr;
> + return c;
> +}
> +
> +/* For operations that can wait for kmalloc to possibly sleep,
> + * this routine can be called. Lock need not be held to call
> + * cmd_special_alloc. cmd_special_free() is the complement.
> + */
> +static struct CommandList_struct *cmd_special_alloc(struct ctlr_info *h)
> +{
> + struct CommandList_struct *c;
> + union u64bit temp64;
> + dma_addr_t cmd_dma_handle, err_dma_handle;
> +
> + c = (struct CommandList_struct *) pci_alloc_consistent(h->pdev,
> + sizeof(struct CommandList_struct), &cmd_dma_handle);
No need to cast a void pointer.
> + if (c == NULL)
> + return NULL;
> + memset(c, 0, sizeof(struct CommandList_struct));
sizeof(*c)
> + c->cmdindex = -1;
> +
> + c->err_info = (struct ErrorInfo_struct *)
> + pci_alloc_consistent(h->pdev, sizeof(struct ErrorInfo_struct),
> + &err_dma_handle);
> +
> + if (c->err_info == NULL) {
> + pci_free_consistent(h->pdev,
> + sizeof(struct CommandList_struct), c, cmd_dma_handle);
> + return NULL;
> + }
> + memset(c->err_info, 0, sizeof(struct ErrorInfo_struct));
Here again.
> + INIT_HLIST_NODE(&c->list);
> + c->busaddr = (__u32) cmd_dma_handle;
> + temp64.val = (__u64) err_dma_handle;
> + c->ErrDesc.Addr.lower = temp64.val32.lower;
> + c->ErrDesc.Addr.upper = temp64.val32.upper;
> + c->ErrDesc.Len = sizeof(struct ErrorInfo_struct);
> +
> + c->ctlr = h->ctlr;
> + return c;
> +}
> +
> +
> +/* Free a command block previously allocated with cmd_alloc(). */
> +static void cmd_free(struct ctlr_info *h, struct CommandList_struct *c)
> +{
> + int i;
> + i = c - h->cmd_pool;
> + clear_bit(i & (BITS_PER_LONG - 1),
> + h->cmd_pool_bits + (i / BITS_PER_LONG));
> + h->nr_frees++;
> +}
> +
> +/* Free a command block previously allocated with cmd_special_alloc(). */
> +static void cmd_special_free(struct ctlr_info *h, struct
> CommandList_struct *c) +{
> + union u64bit temp64;
> +
> + temp64.val32.lower = c->ErrDesc.Addr.lower;
> + temp64.val32.upper = c->ErrDesc.Addr.upper;
> + pci_free_consistent(h->pdev, sizeof(struct ErrorInfo_struct),
> + c->err_info, (dma_addr_t) temp64.val);
sizeof(c->err_info)
> + pci_free_consistent(h->pdev, sizeof(struct CommandList_struct),
> + c, (dma_addr_t) c->busaddr);
sizeof(*c)
I stopped looking for that here, there are maybe some more instances.
> +static int hpsa_pci_init(struct ctlr_info *c, struct pci_dev *pdev)
> +{
> + ushort subsystem_vendor_id, subsystem_device_id, command;
> + __u32 board_id, scratchpad = 0;
> + __u64 cfg_offset;
> + __u32 cfg_base_addr;
> + __u64 cfg_base_addr_index;
> + int i, prod_index, err;
> +
> + subsystem_vendor_id = pdev->subsystem_vendor;
> + subsystem_device_id = pdev->subsystem_device;
> + board_id = (((__u32) (subsystem_device_id << 16) & 0xffff0000) |
> + subsystem_vendor_id);
> +
> + for (i = 0; i < ARRAY_SIZE(products); i++)
> + if (board_id == products[i].board_id)
> + break;
> +
> + prod_index = i;
> +
> + if (prod_index == ARRAY_SIZE(products)) {
> + prod_index--;
> + if (subsystem_vendor_id == !PCI_VENDOR_ID_HP ||
> + !allow_unknown_smartarray) {
> + printk(KERN_WARNING "hpsa: Sorry, I don't "
> + "know how to access the Smart "
> + "Array controller %08lx\n",
> + (unsigned long) board_id);
> + return -ENODEV;
> + }
> + }
> + /* check to see if controller has been disabled */
> + /* BEFORE trying to enable it */
> + (void)pci_read_config_word(pdev, PCI_COMMAND, &command);
> + if (!(command & 0x02)) {
> + printk(KERN_WARNING
> + "hpsa: controller appears to be disabled\n");
> + return -ENODEV;
> + }
> +
> + err = pci_enable_device(pdev);
You may want to use pcim_enable_device() as it moves the work of freeing a
bunch of resources (like pci_request_regions()) to devres and you don't need
to care about this.
Eike
Download attachment "signature.asc " of type "application/pgp-signature" (198 bytes)
Powered by blists - more mailing lists