Commit bf7afb29 authored by Russell King's avatar Russell King Committed by David S. Miller
Browse files

phy: improve safety of fixed-phy MII register reading 



There is no prevention of a concurrent call to both fixed_mdio_read()
and fixed_phy_update_state(), which can result in the state being
modified while it's being inspected.  Fix this by using a seqcount
to detect modifications, and memcpy()ing the state.

We remain slightly naughty here, calling link_update() and updating
the link status within the read-side loop - which would need rework
of the design to change.

Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
Signed-off-by: default avatarRussell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 37688e3f

drivers/net/phy/fixed_phy.c

+21 −7
Original line number Original line Diff line number Diff line
@@ -23,6 +23,7 @@ 
#include <linux/slab.h>

#include <linux/slab.h>

#include <linux/of.h>

#include <linux/of.h>

#include <linux/gpio.h>

#include <linux/gpio.h>

#include <linux/seqlock.h>





#include "swphy.h"

#include "swphy.h"
@@ -34,6 +35,7 @@ struct fixed_mdio_bus { 
struct fixed_phy {

struct fixed_phy {

	int addr;

	int addr;

	struct phy_device *phydev;

	struct phy_device *phydev;

	seqcount_t seqcount;

	struct fixed_phy_status status;

	struct fixed_phy_status status;

	int (*link_update)(struct net_device *, struct fixed_phy_status *);

	int (*link_update)(struct net_device *, struct fixed_phy_status *);

	struct list_head node;

	struct list_head node;
@@ -58,13 +60,21 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num) 




	list_for_each_entry(fp, &fmb->phys, node) {

	list_for_each_entry(fp, &fmb->phys, node) {

		if (fp->addr == phy_addr) {

		if (fp->addr == phy_addr) {

			struct fixed_phy_status state;

			int s;



			do {

				s = read_seqcount_begin(&fp->seqcount);

				/* Issue callback if user registered it. */

				/* Issue callback if user registered it. */

				if (fp->link_update) {

				if (fp->link_update) {

					fp->link_update(fp->phydev->attached_dev,

					fp->link_update(fp->phydev->attached_dev,

							&fp->status);

							&fp->status);

					fixed_phy_update(fp);

					fixed_phy_update(fp);

				}

				}

			return swphy_read_reg(reg_num, &fp->status);

				state = fp->status;

			} while (read_seqcount_retry(&fp->seqcount, s));



			return swphy_read_reg(reg_num, &state);

		}

		}

	}

	}
@@ -116,6 +126,7 @@ int fixed_phy_update_state(struct phy_device *phydev, 




	list_for_each_entry(fp, &fmb->phys, node) {

	list_for_each_entry(fp, &fmb->phys, node) {

		if (fp->addr == phydev->mdio.addr) {

		if (fp->addr == phydev->mdio.addr) {

			write_seqcount_begin(&fp->seqcount);

#define _UPD(x) if (changed->x) \

#define _UPD(x) if (changed->x) \

	fp->status.x = status->x

	fp->status.x = status->x

			_UPD(link);

			_UPD(link);
@@ -125,6 +136,7 @@ int fixed_phy_update_state(struct phy_device *phydev, 
			_UPD(asym_pause);

			_UPD(asym_pause);

#undef _UPD

#undef _UPD

			fixed_phy_update(fp);

			fixed_phy_update(fp);

			write_seqcount_end(&fp->seqcount);

			return 0;

			return 0;

		}

		}

	}

	}
@@ -149,6 +161,8 @@ int fixed_phy_add(unsigned int irq, int phy_addr, 
	if (!fp)

	if (!fp)

		return -ENOMEM;

		return -ENOMEM;





	seqcount_init(&fp->seqcount);



	if (irq != PHY_POLL)

	if (irq != PHY_POLL)

		fmb->mii_bus->irq[phy_addr] = irq;

		fmb->mii_bus->irq[phy_addr] = irq;

CRA Git | Maintained and supported by SUSTech CRA and CCSE