linux-kernel - Re: add Packet hub driver for Topcliff Platform controller hub

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 8 Jan 2013 19:30:34 +0900
From:	Tomoya MORINAGA <tomoya.rohm@...il.com>
To:	Dan Carpenter <dan.carpenter@...cle.com>
Cc:	Arnd Bergmann <arnd@...db.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: add Packet hub driver for Topcliff Platform controller hub

Hi Dan,

On Mon, Jan 7, 2013 at 6:02 PM, Dan Carpenter <dan.carpenter@...cle.com> wrote:
> The patch cf4ece53460c: "add Packet hub driver for Topcliff Platform
> controller hub" from Sep 1, 2010, leads to the following warning:
> drivers/misc/pch_phub.c:596 pch_phub_bin_write()
>          error: buffer overflow 'buf' 4096 <= 15359
>
> Sorry my question is about an old patch.  Smatch complains because we
> only pass a PAGE_SIZE buffer to sysfs files so the test for
> "if (count > PCH_PHUB_OROM_SIZE) {" makes it think we are overflowing.
> In fact, count is never more than 4096 so there is no overflow, but I
> also think that it means only the first 4096 bytes of the firmware gets
> updated.
>
> drivers/misc/pch_phub.c
>    560  static ssize_t pch_phub_bin_write(struct file *filp, struct kobject *kobj,
>    561                                    struct bin_attribute *attr,
>    562                                    char *buf, loff_t off, size_t count)
>    563  {
>    564          int err;
>    565          unsigned int addr_offset;
>    566          int ret;
>    567          ssize_t rom_size;
>    568          struct pch_phub_reg *chip =
>    569                  dev_get_drvdata(container_of(kobj, struct device, kobj));
>    570
>    571          ret = mutex_lock_interruptible(&pch_phub_mutex);
>    572          if (ret)
>    573                  return -ERESTARTSYS;
>    574
>    575          if (off > PCH_PHUB_OROM_SIZE) {
>    576                  addr_offset = 0;
>    577                  goto return_ok;
>    578          }
>    579          if (count > PCH_PHUB_OROM_SIZE) {
>                             ^^^^^^^^^^^^^^^^^^
> This is 15359.
>
>    580                  addr_offset = 0;
>    581                  goto return_ok;
>    582          }
>    583
>    584          chip->pch_phub_extrom_base_address = pci_map_rom(chip->pdev, &rom_size);
>    585          if (!chip->pch_phub_extrom_base_address) {
>    586                  err = -ENOMEM;
>    587                  goto exrom_map_err;
>    588          }
>    589
>    590          for (addr_offset = 0; addr_offset < count; addr_offset++) {
>    591                  if (PCH_PHUB_OROM_SIZE < off + addr_offset)
>    592                          goto return_ok;
>    593
>    594                  ret = pch_phub_write_serial_rom(chip,
>    595                              chip->pch_opt_rom_start_address + addr_offset + off,
>    596                              buf[addr_offset]);
>                                     ^^^^^^^^^^^^^^^^
> Smatch complains because "buf" is only 4096 bytes.
>

I can understand your saying.

You mean just delete the following condition ?

579          if (count > PCH_PHUB_OROM_SIZE) {

Thanks.

-- 
ROHM Co., Ltd.
tomoya
--
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/